df: prefer fullpath entries when deduplicating

* src/df.c (struct devlist): Add a new element for storing
pointers to mount_entry structures.
(devlist_head, dev_examined): Remove.
(filter_mount_list): Add new function to filter out the rootfs
entry (unless -trootfs is specified), and duplicities. The
function favors entries with a '/' character in me_devname
or those with the shortest me_mountdir string, if multiple
entries fulfill the first condition.
Use struct devlist to build up a list of entries already known,
and finally rebuild the global mount_list.
(get_all_entries): Call the above new function unless the -a
option is specified.
(get_dev): Remove the code for skipping rootfs and duplicities.
* tests/df/skip-duplicates.sh: Add test cases.

Co-authored-by: Bernhard Voelker <mail@bernhard-voelker.de>

diff --git a/src/df.c b/src/df.c
index 9523cc16e2fb460563b95aa33d7721e1dea139bd..70dabb5e499417bf6d6257e65025615ef72008a6 100644 (file)
--- a/src/df.c
+++ b/src/df.c
@@ -48,12 +48,10 @@
 struct devlist
 {
   dev_t dev_num;
+  struct mount_entry *me;
   struct devlist *next;
 };
 
-/* Store of already-processed device numbers.  */
-static struct devlist *devlist_head;
-
 /* If true, show even file systems with zero size or
    uninteresting types.  */
 static bool show_all_fs;
@@ -606,27 +604,87 @@ excluded_fstype (const char *fstype)
   return false;
 }
 
-/* Check if the device was already examined.  */
+/* Filter mount list by skipping duplicate entries.
+   In the case of duplicities - based on to the device number - the mount entry
+   with a '/' in its me_devname (i.e. not pseudo name like tmpfs) wins.
+   If both have a real devname (e.g. bind mounts), then that with the shorter
+   me_mountdir wins.
+   Finally, do not filter out a rootfs entry if -trootfs is specified.  */
 
-static bool
-dev_examined (char const *mount_dir, char const *devname)
+static void
+filter_mount_list (void)
 {
-  struct stat buf;
-  if (-1 == stat (mount_dir, &buf))
-    return false;
+  struct mount_entry *me;
 
-  struct devlist *devlist = devlist_head;
-  for ( ; devlist; devlist = devlist->next)
-    if (devlist->dev_num == buf.st_dev)
-      return true;
+  /* Store of already-processed device numbers.  */
+  struct devlist *devlist_head = NULL;
 
-  /* Add the device number to the global list devlist.  */
-  devlist = xmalloc (sizeof *devlist);
-  devlist->dev_num = buf.st_dev;
-  devlist->next = devlist_head;
-  devlist_head = devlist;
+  /* Sort all 'wanted' entries into the list devlist_head.  */
+  for (me = mount_list; me; me = me->me_next)
+    {
+      struct stat buf;
+      struct devlist *devlist;
 
-  return false;
+      if (-1 == stat (me->me_mountdir, &buf))
+        {
+          ;  /* Stat failed - add ME to be able to complain about it later.  */
+        }
+      else
+      if (show_rootfs
+          && (   STREQ (me->me_mountdir, "/")
+              || STREQ (me->me_type, ROOTFS)))
+        {
+          /* Df should show rootfs (due to -trootfs).
+             Add this ME both if it is the rootfs entry itself or "/"
+             (as that must not replace the rootfs entry in the devlist).  */
+          ;
+        }
+      else
+        {
+          /* If the device name is a real path name ...  */
+          if (strchr (me->me_devname, '/'))
+            {
+              /* ... try to find its device number in the devlist.  */
+              for (devlist = devlist_head; devlist; devlist = devlist->next)
+                if (devlist->dev_num == buf.st_dev)
+                  break;
+
+              if (devlist)
+                {
+                  /* Let the shorter mountdir win.  */
+                  if (   !strchr (devlist->me->me_devname, '/')
+                      || ( strlen (devlist->me->me_mountdir)
+                         > strlen (me->me_mountdir)))
+                    {
+                       /* FIXME: free ME - the others are also not free()d.  */
+                      devlist->me = me;
+                    }
+                  continue; /* ... with the loop over the mount_list.  */
+                }
+            }
+        }
+
+      /* Add the device number to the global list devlist.  */
+      devlist = xmalloc (sizeof *devlist);
+      devlist->me = me;
+      devlist->dev_num = buf.st_dev;
+      devlist->next = devlist_head;
+      devlist_head = devlist;
+    }
+
+  /* Finally rebuild the mount_list from the devlist.  */
+  mount_list = NULL;
+  while (devlist_head)
+    {
+      /* Add the mount entry.  */
+      me = devlist_head->me;
+      me->me_next = mount_list;
+      mount_list = me;
+      /* Free devlist entry and advance.  */
+      struct devlist *devlist = devlist_head->next;
+      free (devlist_head);
+      devlist_head = devlist;
+    }
 }
 
 /* Return true if N is a known integer value.  On many file systems,
@@ -798,15 +856,6 @@ get_dev (char const *disk, char const *mount_point,
   if (!selected_fstype (fstype) || excluded_fstype (fstype))
     return;
 
-  if (process_all && !show_all_fs && !show_listed_fs)
-    {
-      /* No arguments nor "df -a", then check if df has to ...  */
-      if (!show_rootfs && STREQ (disk, ROOTFS))
-        return; /* ... skip rootfs: (unless -trootfs is given.  */
-      if (dev_examined (mount_point, disk))
-        return; /* ... skip duplicate entries (bind mounts).  */
-    }
-
   /* If MOUNT_POINT is NULL, then the file system is not mounted, and this
      program reports on the file system that the special file is on.
      It would be better to report on the unmounted file system,
@@ -1133,6 +1182,9 @@ get_all_entries (void)
 {
   struct mount_entry *me;
 
+  if (!show_all_fs)
+    filter_mount_list ();
+
   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, true);
@@ -1506,14 +1558,6 @@ main (int argc, char **argv)
     }
 
   IF_LINT (free (columns));
-  IF_LINT (
-    while (devlist_head)
-      {
-        struct devlist *devlist = devlist_head->next;
-        free (devlist_head);
-        devlist_head = devlist;
-      }
-    );
 
   exit (exit_status);
 }

diff --git a/tests/df/skip-duplicates.sh b/tests/df/skip-duplicates.sh
index 31ab014cb6a980f7743cf3f174638b6e8e7d2565..ad5249b59088b910ff60435083ea852ceec16916 100755 (executable)
--- a/tests/df/skip-duplicates.sh
+++ b/tests/df/skip-duplicates.sh
@@ -39,10 +39,15 @@ struct mntent *getmntent (FILE *fp)
 
   static struct mntent mntent;
 
-  while (done++ < 3)
+  while (done++ < 4)
     {
-      mntent.mnt_fsname = "fsname";
-      mntent.mnt_dir = "/";
+      /* File system - Mounted on
+         fsname       /
+         /fsname      /root
+         /fsname      /
+      */
+      mntent.mnt_fsname = (done == 2) ? "fsname" : "/fsname";
+      mntent.mnt_dir = (done == 3) ? "/root" : "/";
       mntent.mnt_type = "-";
 
       return &mntent;
@@ -65,11 +70,16 @@ test -f x || skip_ "internal test failure: maybe LD_PRELOAD doesn't work?"
 LD_PRELOAD=./k.so df >out || fail=1
 test $(wc -l <out) -eq 2 || { fail=1; cat out; }
 
+# df should also prefer "/fsname" over "fsname"
+test $(grep -c '/fsname' <out) -eq 1 || { fail=1; cat out; }
+# ... and "/fsname" with '/' as Mounted on over '/root'
+test $(grep -c '/root' <out) -eq 0 || { fail=1; cat out; }
+
 # Ensure that filtering duplicates does not affect -a processing.
 LD_PRELOAD=./k.so df -a >out || fail=1
-test $(wc -l <out) -eq 3 || { fail=1; cat out; }
+test $(wc -l <out) -eq 4 || { fail=1; cat out; }
 
-# Ensure that filtering duplcates does not affect
+# Ensure that filtering duplicates does not affect
 # argument processing (now without the fake getmntent()).
 df '.' '.' >out || fail=1
 test $(wc -l <out) -eq 3 || { fail=1; cat out; }