Re: [PATCH] df: avoid stat() for dummy file systems with -l

[Kamil Dudka]

On Wednesday, August 02, 2017 08:10:11 Bernhard Voelker wrote:
> On 08/01/2017 04:49 PM, Kamil Dudka wrote:
> > If I understand it correctly, your patch takes effect regardless the use
> > of -l option.  Then please remove it from the summary to make it clear.
> > 
> > The optimized-out stat() call is, however, not the only change done by
> > your commit.  It has a side-effect that is observable from df's output
> > (and completely unrelated to autofs or remote file systems).  At first
> > glance, the side-effect looks useful but it should be properly analyzed
> > and documented...
> > 
> > Consider the following example:
> > 
> > # mount -t proc none /opt
> > # mount -t tmpfs none /opt
> > 
> > Now unpached df does not list the /opt entry:
> > 
> > # df | grep opt
> > 
> > ... but after your patch, it does:
> > 
> > # df | grep opt
> > none             2005384        0   2005384   0% /opt
> > 
> > Note that it was important that I have used the same "device" in both
> > the mount commands.
> 
> hmm, adding that little condition was too tempting - as there is the
> same one later on in get_dev(). ;-/
> 
> This sounds like there is another bug to fix in filter_mount_list
> wrt over-mounting.
> Maybe it should process the entries from /proc/self/mountinfo from
> the end instead of the beginning: by this it would be easier to filter
> out over-mounted file systems.
> 
> WDYT?

Sounds like it would fix the bug with over-mounted /opt.  But, for some 
reason, it breaks tests/df/skip-duplicates.sh.  This is the patch I tried:

--- a/src/df.c
+++ b/src/df.c
@@ -638,6 +638,24 @@ devlist_free (void *p)
   free (p);
 }

+/* reverse mount_entry list *plist and return its size */
+static int
+reverse_me_list(struct mount_entry **plist)
+{
+  int size = 0;
+  struct mount_entry *me, *next, *prev = NULL;
+  for (me = *plist; me; me = next)
+  {
+    size++;
+    next = me->me_next;
+    me->me_next = prev;
+    prev = me;
+  }
+
+  *plist = prev;
+  return size;
+}
+
 /* Filter mount list by skipping duplicate entries.
    In the case of duplicates - based on the device number - the mount entry
    with a '/' in its me_devname (i.e., not pseudo name like tmpfs) wins.
@@ -652,10 +670,9 @@ filter_mount_list (bool devices_only)

   /* Temporary list to keep entries ordered.  */
   struct devlist *device_list = NULL;
-  int mount_list_size = 0;

-  for (me = mount_list; me; me = me->me_next)
-    mount_list_size++;
+  /* reverse mount_list to better handle over-mounted file systems */
+  int mount_list_size = reverse_me_list(&mount_list);

   devlist_table = hash_initialize (mount_list_size, NULL,
                                  devlist_hash,
@@ -768,6 +785,9 @@ filter_mount_list (bool devices_only)
       hash_free (devlist_table);
       devlist_table = NULL;
   }
+  else
+    /* reverse mount_list back to preserve the order with --all */
+    reverse_me_list(&mount_list);
 }