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 6d926c16018d1406f7477c9e44bc0ed55bf09836 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..7e7b2ba 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
