bug#17833: coreutils 8.22 df

[Bernhard Voelker]

On 07/11/2014 01:28 AM, Pádraig Brady wrote:
> Subject: [PATCH] df: give precedence to real device nodes
> 
> This is significant when /etc/mtab is a real file
> rather than a symlink to /proc/mounts, [...]

In the days we almost start thinking about to switch to
/proc/self/mountinfo for df like mount/util-linux has done long
time ago, I'm not sure if we should support such old (or broken?)
systems.
Presumably the version of mount is also quite old on such a system,
and now we would add complexity to current df to work around the
problems of such old mount?
After all, it's just one stat() more, so I'm 50:50.

> diff --git a/src/df.c b/src/df.c
> index 063cabf..46aeef9 100644
> --- a/src/df.c
> +++ b/src/df.c
> @@ -48,6 +48,7 @@
>  static struct devlist
>  {
>    dev_t dev_num;
> +  bool real_dev;
>    struct mount_entry *me;
>    struct devlist *next;
>  } *device_list;
> @@ -618,35 +619,46 @@ filter_mount_list (bool devices_only)
>    /* Sort all 'wanted' entries into the list device_list.  */
>    for (me = mount_list; me;)
>      {
> -      struct stat buf;
> +      struct stat mnt_buf;
>        struct devlist *devlist;
>        struct mount_entry *discard_me = NULL;
> +      bool real_dev = false;
>  
>        /* TODO: On Linux we might avoid this stat() and another in get_dev()
>           by using the device IDs available from /proc/self/mountinfo.
>           read_file_system_list() could populate me_dev from those
>           for efficiency and accuracy.  */
> -      if (-1 == stat (me->me_mountdir, &buf))
> +      if (-1 == stat (me->me_mountdir, &mnt_buf))
>          {
>            /* Stat failed - add ME to be able to complain about it later.  */
> -          buf.st_dev = me->me_dev;
> +          mnt_buf.st_dev = me->me_dev;
>          }
>        else
>          {
> +          /* when /etc/mtab is not linked to /proc/mounts,
> +             then in the presence of bind mounts, the source will
> +             be the bind mounted directory, rather than the base device.
> +             In this case we want to give precedence to the base device.  */
> +          struct stat sb;
> +          real_dev = (! me->me_dummy
> +                      && stat (me->me_devname, &sb) == 0
> +                      && (S_ISBLK (sb.st_mode) || S_ISCHR (sb.st_mode)));
> +
>            /* If we've already seen this device...  */
>            for (devlist = device_list; devlist; devlist = devlist->next)
> -            if (devlist->dev_num == buf.st_dev)
> +            if (devlist->dev_num == mnt_buf.st_dev)
>                break;
>  
>            if (devlist)
>              {
>                /* ...let the shorter mountdir win.  */
> -              if ((strchr (me->me_devname, '/')
> -                   && ! strchr (devlist->me->me_devname, '/'))
> -                  || (strlen (devlist->me->me_mountdir)
> -                      > strlen (me->me_mountdir))
> -                  /* or one overmounted on a different device.  */
> -                  || ! STREQ (devlist->me->me_devname, me->me_devname))
> +              if (real_dev >= devlist->real_dev
> +                  && ((strchr (me->me_devname, '/')
> +                       && ! strchr (devlist->me->me_devname, '/'))
> +                      || (strlen (devlist->me->me_mountdir)
> +                          > strlen (me->me_mountdir))
> +                      /* or one overmounted on a different device.  */
> +                      || ! STREQ (devlist->me->me_devname, me->me_devname)))

Wow, I never tried to use >= operator on a bool type, but it's
- although IMO not very readable - obviously permitted and working.

>                  {
>                    /* Discard mount entry for existing device.  */
>                    discard_me = devlist->me;
> @@ -672,7 +684,8 @@ filter_mount_list (bool devices_only)
>            /* Add the device number to the global list devlist.  */
>            devlist = xmalloc (sizeof *devlist);
>            devlist->me = me;
> -          devlist->dev_num = buf.st_dev;
> +          devlist->dev_num = mnt_buf.st_dev;
> +          devlist->real_dev = real_dev;
>            devlist->next = device_list;
>            device_list = devlist;
>  
> -- 1.7.7.6

With all the recent patches it seems like the upcoming refactoring
of df.c could get some hard work ... or fun (depending on how one
may want to put it). ;-)

Thanks & have a nice day,
Berny