On 11/09/15 22:55, Dave Chiluk wrote: > On 09/11/2015 03:42 PM, Pádraig Brady wrote: >> On 31/08/15 22:05, 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..00c77c1 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. */ >>> + /* Only if it's 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)))) >> >> As previously mentioned, should this be >= ? > Sorry my apologies for not addressing that. I don't think it should be > (strlen (devlist->me->me_mntroot)>= strlen(me->me_mntroot)), because > that would give preference to later mounts of the same me_mntroot > directory *(I'm specifically thinking of the case where both me_mntroot > = "/"). > > However, looking at the code again, I realized that by adding the NULL > checks I have actually changed the behavior for non-linux OS's since the > NULL checks would always defeat the (strlen (devlist->me->me_mountdir) > > strlen (me->me_mountdir) check. > > I'm going to think through this nasty if case and see if I can make it > prettier. > >> >> We'll need to look at tests. >> This will be quite tricky given the current >> avoidance of /proc/self/mountinfo in tests. >> I'll look at adding a separate test for this. >> >> thanks, >> Pádraig. >> > > Additionally I agree with you that we should mirror the find_mnt > bracketting behavior. > /dev/sdb2 xfs 12.7G 9.4G 3.3G 74% / > /dev/sdb2[/etc] xfs /root/chroot > > I'll submit that in a different patch once we have this more squared away.
Well I mentioned that df was "DEVICE oriented" and so should probably not worry about distinguishing that, especially as the functionality is already provided by findmnt(1). thanks, Pádraig
