Subject: Re: bug#21372: [PATCH v4] df: fix prioritize real mounts over bind mounts

From: Dave Chiluk <[email-masked]>
Date: Tue, 22 Sep 2015 17:28:17 -0500

The patch I just sent out broke existing behavior on non-linux again.
I'm really beginning to hate that if statement.  Anyhow, I fixed that,
and changed it around for readability.

Dave.

On 09/22/2015 05:03 PM, Dave Chiluk wrote:
> I like and appreciate the refactor, but I think the variable name
> "target_nearer_device_root" for the boolean, follows the existing code
> better.  I also fixed the logic to match the new name.
> 
> Dave.
> 
> On 09/21/2015 07:43 PM, Pádraig Brady wrote:
>> On 21/09/15 23:44, Pádraig Brady wrote:
>>> On 21/09/15 21:04, Dave Chiluk wrote:
>>>> Fixes an issue where bind mounts with shorter mount directories than the
>>>> original mount are prioritized when running df.  The root cause of this
>>>> is that /proc/self/mountinfo now lists the filesystem device with bind
>>>> mounts rather than the source directory. With /etc/mtab the source
>>>> device was listed as the originating directory so this was not an issue.
>>>>
>>>> More information is available here.
>>>> https://bugs.launchpad.net/ubuntu/+source/coreutils/+bug/1432871
>>>> ---
>>>>  src/df.c | 9 +++++++--
>>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/df.c b/src/df.c
>>>> index 2e541b9..4a3afa2 100644
>>>> --- a/src/df.c
>>>> +++ b/src/df.c
>>>> @@ -652,9 +652,14 @@ filter_mount_list (bool devices_only)
>>>>                else if ((strchr (me->me_devname, '/')
>>>>                         /* let "real" devices with '/' in the name win.  */
>>>>                          && ! strchr (devlist->me->me_devname, '/'))
>>>> -                       /* let a shorter mountdir win.  */
>>>> -                       || (strlen (devlist->me->me_mountdir)
>>>> +                       /* let a shorter mountdir win. If it's mntroot is */
>>>> +                       /* also shorter. i.e. not a bind mount. */
>>>> +                       || ((strlen (devlist->me->me_mountdir)
>>>>                             > strlen (me->me_mountdir))
>>>> +                          && (devlist->me->me_mntroot == NULL
>>>> +                             || me->me_mntroot == NULL
>>>> +                             || (strlen (devlist->me->me_mntroot)
>>>> +                                 >= strlen(me->me_mntroot))))
>>>>                         /* let an entry overmounted on a new device win...  */
>>>>                         || (! STREQ (devlist->me->me_devname, me->me_devname)
>>>>                             /* ... but only when matching an existing mnt point,
>>>>
>>>
>>> This logic looks correct thanks.
>>> I'll adjust the comment a bit and maybe refactor
>>> all the conditions to a single nearer_device_root boolean.
>>>
>>> I'll push this later on,
>>> with the associated gnulib update.
>>
>> Attached is the refactored patch,
>> with also lots of adjustments to commit messages.
>> I'll apply tomorrow.
>>
>> thanks,
>> Pádraig.
>>
> 

>From d477261ec951ca51c3c85b08cd466b8cf24ae27a Mon Sep 17 00:00:00 2001
From: Dave Chiluk <[email-masked]>
Date: Mon, 21 Sep 2015 15:04:11 -0500
Subject: [PATCH] df: prioritize mounts nearer the device root

In the presence of bind mounts of a device, the 4th "mount root" field
from /proc/self/mountinfo is now considered, so as to prefer mount
points closer to the root of the device.  Note on older systems with
an /etc/mtab file, the source device was listed as the originating
directory, and so this was not an issue.
Details at http://pad.lv/1432871

* src/df.c (filter_mount_list): When deduplicating mount entries,
only prefer sources nearer or at the root of the device, when the
target is nearer the root of the device.
* NEWS: Mention the change in behavior.
---
 NEWS     |  3 +++
 src/df.c | 12 +++++++++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/NEWS b/NEWS
index 63574da..9aec259 100644
--- a/NEWS
+++ b/NEWS
@@ -17,6 +17,9 @@ GNU coreutils NEWS                                    -*- outline -*-
   base64 no longer supports hex or oct --wrap parameters,
   thus better supporting decimals with leading zeros.
 
+  df now prefers sources towards the root of a device when
+  eliding duplicate bind mounted entries.
+
 ** Improvements
 
   du no longer stats all mount points at startup, only doing so
diff --git a/src/df.c b/src/df.c
index 2e541b9..b96155e 100644
--- a/src/df.c
+++ b/src/df.c
@@ -641,6 +641,13 @@ filter_mount_list (bool devices_only)
 
           if (devlist)
             {
+              bool target_nearer_root = strlen (devlist->me->me_mountdir)
+                                        > strlen (me->me_mountdir);
+              /* With bind mounts, prefer items nearer the root of the device */
+              bool target_nearer_device_root = ! (devlist->me->me_mntroot != NULL
+                                       && me->me_mntroot != NULL
+                                       && (strlen (devlist->me->me_mntroot)
+                                           < strlen(me->me_mntroot)));
               if (! print_grand_total && me->me_remote && devlist->me->me_remote
                   && ! STREQ (devlist->me->me_devname, me->me_devname))
                 {
@@ -652,9 +659,8 @@ filter_mount_list (bool devices_only)
               else if ((strchr (me->me_devname, '/')
                        /* let "real" devices with '/' in the name win.  */
                         && ! strchr (devlist->me->me_devname, '/'))
-                       /* let a shorter mountdir win.  */
-                       || (strlen (devlist->me->me_mountdir)
-                           > strlen (me->me_mountdir))
+                       /* let points towards the root of the device win.  */
+                       || (target_nearer_root && target_nearer_device_root)
                        /* let an entry overmounted on a new device win...  */
                        || (! STREQ (devlist->me->me_devname, me->me_devname)
                            /* ... but only when matching an existing mnt point,
-- 
2.5.0