Re: bug#10363: df vs. long-named /dev/disk/by-uuid/... file system device names

[Jim Meyering]

Jim Meyering wrote:

> Pádraig Brady wrote:
> ...
>>> Now, I'm thinking about making this a little more future-proof by
>>> matching the UUID part /[0-9a-fA-F-]{36}$/ instead.
>>> I.e., testing something like this:
>>>
>>>      if (process_all
>>>          && has_uuid_suffix (dev_name)
>>>          && (resolved_dev = canonicalize_filename_mode (dev_name, 
>>> CAN_EXISTING)))
>>>
>>> using this new function:
>>>
>>>   static bool
>>>   has_uuid_suffix (char const *s)
>>>   {
>>>     size_t len = strlen (s);
>>>     return (36 < len
>>>             && strspn (s + len - 36, "-0123456789abcdefABCDEF") == 36);
>>>   }
>>
>> Yes that's awkward but warranted.
>> The logic looks correct.
>>
>> This is assuming of course that UUIDs are the only "high level" form
>> presented in /proc/mounts that one doesn't want displayed.
>> I can't think of anything else worth avoiding at the moment.
>
> Thanks for the review.
> Here's an updated patch.
> Changes:
>   - added comment for new function
>   - added _GL_ATTRIBUTE_PURE, too
>   - updated commit log and comments
>
>>From b30f021ada261e8b248f7040f8c98db6495e9e88 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <address@hidden>
> Date: Thu, 29 Dec 2011 14:49:00 +0100
> Subject: [PATCH] df: work around long-named /dev/disk/by-uuid/... symlinks

I've added a NEWS entry:

>From 1e18d8416f9ef43bf08982cabe54220587061a08 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Thu, 29 Dec 2011 14:49:00 +0100
Subject: [PATCH] df: work around long-named /dev/disk/by-uuid/... symlinks

On systems with recent kernel/tools, a symlink from /etc/mtab to
/proc/mounts, and a by-UUID mount (i.e., soon, nearly everyone),
you will see something like the following when running "df -hT":
(this has been truncated to fit in a width-limited ChangeLog file)

Filesystem                                             Type      Siz...
rootfs                                                 rootfs     11G
udev                                                   devtmpfs  3.8G
tmpfs                                                  tmpfs     774M
/dev/disk/by-uuid/828fc648-9f30-43d8-a0b1-f7096a2edb66 ext4       11G
tmpfs                                                  tmpfs     1.6G
/dev/sda2                                              ext3      494M
/dev/sda5                                              ext4       12G
/dev/sda6                                              ext4      9.9G

Contrast that with what we're used to seeing (modulo the
two entries mounted on "/", which is a separate problem):

Filesystem     Type      Size  Used Avail Use% Mounted on
rootfs         rootfs     11G  1.9G  8.0G  19% /
udev           devtmpfs  3.8G     0  3.8G   0% /dev
tmpfs          tmpfs     774M  376K  774M   1% /run
/dev/sda3      ext4       11G  1.9G  8.0G  19% /
tmpfs          tmpfs     1.6G  8.0K  1.6G   1% /run/shm
/dev/sda2      ext3      494M   78M  392M  17% /boot
/dev/sda5      ext4       12G  7.6G  3.7G  68% /usr
/dev/sda6      ext4      9.9G  6.6G  2.8G  71% /var

When that long /dev/disk/by-uuid/... name is merely a symlink
to a much shorter (and often more useful) device name like
"/dev/sda3", and when it's part of a listing of all file systems,
I would much prefer to see only the latter.  Similarly, when using
an encrypted root file system, you would see a name like
/dev/mapper/luks-828fc648-9f30-43d8-a0b1-f7196a2edb66 pointing
to say, /dev/dm-0, I prefer the shorter name.

I.e., if I explicitly run
"df -hT /dev/disk/by-uuid/828fc648-9f30-43d8-a0b1-f7096a2edb66",
then, it's fine -- and expected -- to print to the long name.
It was explicitly given.  However, with no non-option argument,
df should print the shorter name.  Note that performing this
translation at a lower level (via a change to gnulib's mountlist.c)
would make it impossible to distinguish those two cases.

* src/df.c: Include "canonicalize.h".
(get_dev): Add a parameter, telling when we're in process-all-
mount-points mode; update all callers.  When true, resolve
UUID-suffixed symlinks.
* NEWS (Changes in behavior): Mention it.
Reported by Dan Jacobson in http://bugs.gnu.org/10363
---
 NEWS     |    5 +++++
 src/df.c |   49 +++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/NEWS b/NEWS
index 78a90b6..bc5a0a9 100644
--- a/NEWS
+++ b/NEWS
@@ -36,6 +36,11 @@ GNU coreutils NEWS                                    -*- 
outline -*-

 ** Changes in behavior

+  df, with no non-option argument and recent enough kernel/tools, would
+  print a long UUID-including file system name, pushing second and subsequent
+  columns far to the right.  Now, when that long name refers to a symlink,
+  df prints the usually-short referent instead.
+
   tail -f now uses polling (not inotify) when any of its file arguments
   resides on a file system of unknown type.  In addition, for each such
   argument, tail -f prints a warning with the FS type magic number and a
diff --git a/src/df.c b/src/df.c
index 9677687..fae32cd 100644
--- a/src/df.c
+++ b/src/df.c
@@ -25,6 +25,7 @@
 #include <assert.h>

 #include "system.h"
+#include "canonicalize.h"
 #include "error.h"
 #include "fsusage.h"
 #include "human.h"
@@ -417,6 +418,17 @@ add_uint_with_neg_flag (uintmax_t *dest, bool *dest_neg,
     *dest = -*dest;
 }

+/* Return true if S ends in a string that may be a 36-byte UUID,
+   i.e., of the form HHHHHHHH-HHHH-HHHH-HHHH-HHHHHHHHHHHH, where
+   each H is an upper or lower case hexadecimal digit.  */
+static bool _GL_ATTRIBUTE_PURE
+has_uuid_suffix (char const *s)
+{
+  size_t len = strlen (s);
+  return (36 < len
+          && strspn (s + len - 36, "-0123456789abcdefABCDEF") == 36);
+}
+
 /* Obtain a space listing for the disk device with absolute file name DISK.
    If MOUNT_POINT is non-NULL, it is the name of the root of the
    file system on DISK.
@@ -428,13 +440,16 @@ add_uint_with_neg_flag (uintmax_t *dest, bool *dest_neg,
    If FSTYPE is non-NULL, it is the type of the file system on DISK.
    If MOUNT_POINT is non-NULL, then DISK may be NULL -- certain systems may
    not be able to produce statistics in this case.
-   ME_DUMMY and ME_REMOTE are the mount entry flags.  */
+   ME_DUMMY and ME_REMOTE are the mount entry flags.
+   Caller must set PROCESS_ALL to true when iterating over all entries, as
+   when df is invoked with no non-option argument.  See below for details.  */

 static void
 get_dev (char const *disk, char const *mount_point,
          char const *stat_file, char const *fstype,
          bool me_dummy, bool me_remote,
-         const struct fs_usage *force_fsu)
+         const struct fs_usage *force_fsu,
+         bool process_all)
 {
   struct fs_usage fsu;
   char buf[LONGEST_HUMAN_READABLE + 2];
@@ -488,6 +503,24 @@ get_dev (char const *disk, char const *mount_point,

   if (! disk)
     disk = "-";                        /* unknown */
+
+  char *dev_name = xstrdup (disk);
+  char *resolved_dev;
+
+  /* On some systems, dev_name is a long-named symlink like
+     /dev/disk/by-uuid/828fc648-9f30-43d8-a0b1-f7196a2edb66 pointing to a
+     much shorter and more useful name like /dev/sda1.  It may also look
+     like /dev/mapper/luks-828fc648-9f30-43d8-a0b1-f7196a2edb66 and point to
+     /dev/dm-0.  When process_all is true and dev_name is a symlink whose
+     name ends with a UUID use the resolved name instead.  */
+  if (process_all
+      && has_uuid_suffix (dev_name)
+      && (resolved_dev = canonicalize_filename_mode (dev_name, CAN_EXISTING)))
+    {
+      free (dev_name);
+      dev_name = resolved_dev;
+    }
+
   if (! fstype)
     fstype = "-";              /* unknown */

@@ -537,7 +570,7 @@ get_dev (char const *disk, char const *mount_point,
       switch (field)
         {
         case DEV_FIELD:
-          cell = xstrdup (disk);
+          cell = dev_name;
           break;

         case TYPE_FIELD:
@@ -648,7 +681,7 @@ get_disk (char const *disk)
     {
       get_dev (best_match->me_devname, best_match->me_mountdir, NULL,
                best_match->me_type, best_match->me_dummy,
-               best_match->me_remote, NULL);
+               best_match->me_remote, NULL, false);
       return true;
     }

@@ -734,7 +767,7 @@ get_point (const char *point, const struct stat *statp)
   if (best_match)
     get_dev (best_match->me_devname, best_match->me_mountdir, point,
              best_match->me_type, best_match->me_dummy, best_match->me_remote,
-             NULL);
+             NULL, false);
   else
     {
       /* We couldn't find the mount entry corresponding to POINT.  Go ahead and
@@ -745,7 +778,7 @@ get_point (const char *point, const struct stat *statp)
       char *mp = find_mount_point (point, statp);
       if (mp)
         {
-          get_dev (NULL, mp, NULL, NULL, false, false, NULL);
+          get_dev (NULL, mp, NULL, NULL, false, false, NULL, false);
           free (mp);
         }
     }
@@ -774,7 +807,7 @@ get_all_entries (void)

   for (me = mount_list; me; me = me->me_next)
     get_dev (me->me_devname, me->me_mountdir, NULL, me->me_type,
-              me->me_dummy, me->me_remote, NULL);
+             me->me_dummy, me->me_remote, NULL, true);
 }

 /* Add FSTYPE to the list of file system types to display. */
@@ -1066,7 +1099,7 @@ main (int argc, char **argv)
     {
       if (inode_format)
         grand_fsu.fsu_blocks = 1;
-      get_dev ("total", NULL, NULL, NULL, false, false, &grand_fsu);
+      get_dev ("total", NULL, NULL, NULL, false, false, &grand_fsu, false);
     }

   print_table ();
--
1.7.8.1.391.g2c2ad