[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: df command should suppress duplicates
From: |
Bernhard Voelker |
Subject: |
Re: df command should suppress duplicates |
Date: |
Thu, 16 Aug 2012 08:43:08 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:14.0) Gecko/20120713 Thunderbird/14.0 |
On 08/15/2012 05:41 PM, Ondrej Oprala wrote:
> Hi,
> I've written a little fix for this bugzilla
> https://bugzilla.redhat.com/show_bug.cgi?id=709351
> which I think could solve the duplicity output problem. The idea is that
> df would keep a linked
> list of device numbers it already went through and not count in
> (get_dev) those filesystems again
> when running into them as bind mounts.
> Cheers,
> Ondrej.
Thanks for working on that.
> +/* Global device number linked list. */
> +static struct devlist *devlist;
> +
> +/* Help pointer to devlist's first entry. */
> +static struct devlist *devlist_head;
> +
I think we don't need both as static global variable.
The former is only used as a loop variable, so I'd define
it where it's needed.
> +/* Add a device number of the soon-to-be examined
> + filesystem to the global list devlist. */
> +
> +static void
> +devlist_add (dev_t dev_num)
> +{
> + devlist->dev_num = dev_num;
> + devlist->dev_next = xmalloc (sizeof *devlist);
> + devlist = devlist->dev_next;
> + devlist->dev_num = 0;
> + devlist->dev_next = NULL;
> + devlist = devlist_head;
> +}
Together with the new code in main, you allocate one item
more than what is needed. If the loop in dev_examinded()
would run "while (devlist)", then this wouldn't be necessary.
> +/* Check if the device was already examined. */
> +
> +static bool
> +dev_examined (char *mount_dir)
> +{
> + dev_t dev_num;
> + struct stat *buf = alloca (sizeof *buf);
Isn't the use of alloca discouraged? A simple "struct stat buf"
would be sufficient.
> + stat (mount_dir, buf);
> +
> + dev_num = buf->st_dev;
> + while (devlist->dev_num)
> + {
> + if (devlist->dev_num == dev_num)
> + {
> + devlist = devlist_head;
> + return true;
> + }
> + devlist = devlist->dev_next;
> + }
> +
> + devlist_add (dev_num);
> + return false;
> +}
...
> @@ -830,8 +882,11 @@ get_all_entries (void)
> struct mount_entry *me;
>
> for (me = mount_list; me; me = me->me_next)
> - get_dev (me->me_devname, me->me_mountdir, NULL, me->me_type,
> - me->me_dummy, me->me_remote, NULL, true);
> + if (!dev_examined (me->me_mountdir))
> + {
> + get_dev (me->me_devname, me->me_mountdir, NULL, me->me_type,
> + me->me_dummy, me->me_remote, NULL, true);
> + }
> }
...
> @@ -1132,7 +1187,21 @@ main (int argc, char **argv)
> get_entry (argv[i], &stats[i - optind]);
> }
> else
> - get_all_entries ();
> + {
> + devlist = xmalloc (sizeof *devlist);
> + devlist->dev_num = 0;
> + devlist->dev_next = NULL;
> + devlist_head = devlist;
See my comment above: allocating one more than actually needed.
> +
> + get_all_entries ();
> +
> + while (devlist_head)
> + {
> + devlist = devlist_head->dev_next;
> + free (devlist_head);
> + devlist_head = devlist;
> + }
> + }
>
> if (print_grand_total && file_systems_processed)
> {
There's no test, although I personally think it's hard to
write a test ... well, maybe by faking /etc/mtab, i.e. by
intercepting getmntent() in another LD_PRELOAD'ed test? ;-)
The practical result is good, although - as a user - I'd expect
that the "rootfs" line would disappear instead of the line showing
the device which is mounted on '/'. E.g. on my PC, I now see
rootfs 12G 7.2G 3.9G 66% /
instead of
/dev/sda1 12G 7.2G 3.9G 66% /
Even the new findmnt command of util-linux shows the device:
$ ~/ul/findmnt --df
SOURCE FSTYPE SIZE USED AVAIL USE% TARGET
...
/dev/sda1 ext4 11.5G 7.1G 4.4G 62% /
...
Have a nice day,
Berny