[gparted] Support /etc/fstab using Unicode labels as mount points (#786502)



commit 73fe1dbf5cc95eff057bcd7f56ff2b1a710169ec
Author: Mike Fleetwood <mike fleetwood googlemail com>
Date:   Mon Aug 28 10:52:44 2017 +0100

    Support /etc/fstab using Unicode labels as mount points (#786502)
    
    So far GParted is still loading the default non-reversible encoded
    labels from blkid in the initial loading of the FS_Info module cache.
    This encoded label is used to match LABEL=<label> when reading
    /etc/fstab, via the get_path_by_label() call, so works for ASCII only
    labels.  This prevents GParted enabling the "mount on >" partition menu
    item when non-ASCII labels are used.
    
    To fix this:
    1) Stop reading the labels the wrong way.
       Via the blkid command used to initially load the FS_Info module cache
       and is subject to default non-reversible encoding of non-printable
       ASCII bytes.
    2) Read all the labels the right way, but only when needed.
       Only when /etc/fstab file contains LABEL=<label> and
       get_path_by_label() is called, read all the labels from blkid without
       encoding them via run_blkid_update_cache_one_label().
    3) Return label from the cache.
       get_label() returns the cached label, loading it into the cache first
       if needed with run_blkid_update_cache_one_label().
    
    In the worst case scenario of having a LABEL=<label> in /etc/fstab blkid
    will be run for every partition containing a recognised file system to
    read the label.  On my desktop with 5 hard drives, 4  SWRaid arrays and
    31 recognised file systems running 'blkid -o value -s LABEL ...' 31
    times took 0.074 seconds of a total scan time of 9.072 seconds.  Less
    that 1% of the total scanning time.  When LABEL=<label> is not used in
    /etc/fstab individual blkid executions are only used to read labels for
    file systems where there is no file system specific tool available
    reducing the impact further.  Blkid itself caches the data in it's
    blkid.tab cache file rather than reading all file systems on each
    invocation.  Also the Linux file system cache will already contain the
    blkid executable file, needed libraries files and the blkid.tab cache
    file itself.  Hence why repeated execution of blkid is so fast.
    
    Further to the updated comment in set_partition_label_and_uuid().
    Matching LABEL=<label> from /etc/fstab uses the label obtained from
    blkid run in the C locale so this kind of assumes it returns the label
    correctly and it does for my limited testing on Unicode enabled
    desktops.  Just not sure if it would be true for all cases in all
    locales compared to the FS specific command run in the users default
    locale.
    
    Bug 786502 - Support reading Unicode labels when file system specific
                 tools aren't available

 include/FS_Info.h   |    1 +
 src/FS_Info.cc      |   34 ++++++++++++++++++++++------------
 src/GParted_Core.cc |    8 ++++++--
 3 files changed, 29 insertions(+), 14 deletions(-)
---
diff --git a/include/FS_Info.h b/include/FS_Info.h
index 472b74a..d1ecf72 100644
--- a/include/FS_Info.h
+++ b/include/FS_Info.h
@@ -52,6 +52,7 @@ private:
        static void load_fs_info_cache();
        static void load_fs_info_cache_extra_for_path( const Glib::ustring & path );
        static bool run_blkid_load_cache( const Glib::ustring & path = "" );
+       static void update_fs_info_cache_all_labels();
        static bool run_blkid_update_cache_one_label( FS_Entry & fs_entry );
 
        static bool fs_info_cache_initialized ;
diff --git a/src/FS_Info.cc b/src/FS_Info.cc
index ec4d783..bdcbc7c 100644
--- a/src/FS_Info.cc
+++ b/src/FS_Info.cc
@@ -93,13 +93,14 @@ Glib::ustring FS_Info::get_label( const Glib::ustring & path, bool & found )
        for ( unsigned int i = 0 ; i < fs_info_cache.size() ; i ++ )
                if ( bs == fs_info_cache[i].path )
                {
-                       if ( fs_info_cache[i].type == "" )
+                       if ( fs_info_cache[i].have_label || fs_info_cache[i].type == "" )
                        {
-                               // This is a blank cache entry for a whole disk device
-                               // containing a partition table, so no label (as created
-                               // by load_fs_info_cache_extra_for_path()).
-                               found = false;
-                               return "";
+                               // Already have the label or this is a blank cache entry
+                               // for a whole disk device containing a partition table,
+                               // so no label (as created by
+                               // load_fs_info_cache_extra_for_path()).
+                               found = fs_info_cache[i].have_label;
+                               return fs_info_cache[i].label;
                        }
 
                        // Run blkid to get the label for this one partition, update the
@@ -134,6 +135,7 @@ Glib::ustring FS_Info::get_path_by_uuid( const Glib::ustring & uuid )
 Glib::ustring FS_Info::get_path_by_label( const Glib::ustring & label )
 {
        initialize_if_required();
+       update_fs_info_cache_all_labels();
        for ( unsigned int i = 0 ; i < fs_info_cache.size() ; i ++ )
                if ( label == fs_info_cache[i].label )
                        return fs_info_cache[i].path.m_name;
@@ -224,7 +226,8 @@ void FS_Info::load_fs_info_cache_extra_for_path( const Glib::ustring & path )
 bool FS_Info::run_blkid_load_cache( const Glib::ustring & path )
 {
        // Parse blkid output line by line extracting mandatory field: path and optional
-       // fields: type, sec_type, uuid, label.
+       // fields: type, sec_type, uuid.  Label is not extracted here because of blkid's
+       // default non-reversible encoding of non printable ASCII bytes.
        // Example output:
        //     /dev/sda1: UUID="f828ee8c-1e16-4ca9-b234-e4949dcd4bd1" TYPE="xfs"
        //     /dev/sda2: UUID="p31pR5-qPLm-YICz-O09i-sB4u-mAH2-GVSNWG" TYPE="LVM2_member"
@@ -255,11 +258,6 @@ bool FS_Info::run_blkid_load_cache( const Glib::ustring & path )
                                fs_entry.type = Utils::regexp_label( lines[i], " TYPE=\"([^\"]*)\"" );
                                fs_entry.sec_type = Utils::regexp_label( lines[i], " SEC_TYPE=\"([^\"]*)\"" );
                                fs_entry.uuid = Utils::regexp_label( lines[i], " UUID=\"([^\"]*)\"" );
-                               if ( lines[i].find( " LABEL=\"" ) != Glib::ustring::npos )
-                               {
-                                       fs_entry.have_label = true;
-                                       fs_entry.label = Utils::regexp_label( lines[i], " LABEL=\"([^\"]*)\"" 
);
-                               }
                                fs_info_cache.push_back( fs_entry );
                                loaded_entries = true;
                        }
@@ -269,6 +267,18 @@ bool FS_Info::run_blkid_load_cache( const Glib::ustring & path )
        return loaded_entries;
 }
 
+void FS_Info::update_fs_info_cache_all_labels()
+{
+       if ( ! blkid_found )
+               return;
+
+       // For all cache entries which are file systems but don't yet have a label load it
+       // now.
+       for ( unsigned int i = 0 ; i < fs_info_cache.size() ; i ++ )
+               if ( fs_info_cache[i].type != "" && ! fs_info_cache[i].have_label )
+                       run_blkid_update_cache_one_label( fs_info_cache[i] );
+}
+
 bool FS_Info::run_blkid_update_cache_one_label( FS_Entry & fs_entry )
 {
        if ( ! blkid_found )
diff --git a/src/GParted_Core.cc b/src/GParted_Core.cc
index d386168..76b6c03 100644
--- a/src/GParted_Core.cc
+++ b/src/GParted_Core.cc
@@ -1284,8 +1284,12 @@ void GParted_Core::set_partition_label_and_uuid( Partition & partition )
                return;
        }
 
-       // Retrieve file system label.  Use file system specific method first in an effort
-       // to ensure multi-byte character sets are properly displayed.
+       // Retrieve file system label.  Use file system specific method first because it
+       // has been that way since (#662537) to display Unicode labels correctly.
+       // (#786502) adds support for reading Unicode labels through the FS_Info cache.
+       // Either method should produce the same labels however the FS specific command is
+       // run in the users locale where as blkid is run in the C locale.  Shouldn't
+       // matter but who knows for sure!
        read_label( partition );
        if ( ! partition.filesystem_label_known() )
        {


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]