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 protected]> 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
