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
DIFF