On 11/29/2012 06:45 PM, Bernhard Voelker wrote:
On 10/04/2012 07:36 PM, Ondrej Oprala wrote:
Hi, I haven't heard from this thread in a while, are there still things
to be done here?
Hi Ondrej,

FWIW, I've rebased your patch to Git master.
(The only change I made - besides resolving the conflicts -
was to declare devlist_head static to make syntax-check happy.)

However, that implementation needs further discussion:

* IMO, "df -a" should not skip bind mounts, and instead
   print all of the lines.

* It doesn't play well with Robert's suggestion for hiding
   the 'rootfs' entry, because dev_examined() already added
   the entry for the '/' file system before the rootfs entry is
   skipped, which makes the real '/' entry also be skipped.
   http://lists.gnu.org/archive/html/coreutils/2012-11/msg00091.html

   But also Robert's patch deserves further discussion.
   IMO e.g. "df -t rootfs" and again "df -a" should not skip
   rootfs. And read_file_system_list must be called with true
   param to make it work.

* It still allocates one devlist entry more than needed.

Have a nice day,
Berny
Hi Bernhard,
thanks for the rebase :) .
I've modified the patch a bit, so I dont interfere with output if df -a is specified.

Regarding Robert's suggestion/patch and your opinions on it,
I agree that df -t rootfs should indeed output info about it.
So rootfs is now filtered as well as usual duplicates by default,
but should show up with the -[at] options.

As for the extra entry...sorry but I don't see it, the first entry (allocated
in get_all_entries) is filled/used as well as all the others.

Thanks,
Ondrej.



From 85972b2cd7f92a1252177c1f6c140ebcae2a7e04 Mon Sep 17 00:00:00 2001
From: Ondrej Oprala <[email protected]>
Date: Mon, 3 Dec 2012 17:52:06 +0100
Subject: [PATCH] df: do not print duplicate entries and rootfs by default

*NEWS: Mention the fix.
*src/df.c: (devlist_add): Add a new entry to the devlist.
(dev_examined): Check if the device has already been traversed.
(get_all_entries): Change the function's execution so that
it filters duplicit entries, unless the -a option is specified.
(main): Set the show_rootfs variable appropriately when
the -t option is specified.
*tests/df/df-dup-remove.sh: Add a new test to exercise the changes
*tests/local.mk: Add a test entry.
---
 NEWS                      |  3 ++
 src/df.c                  | 87 +++++++++++++++++++++++++++++++++++++++++++++--
 tests/df/df-dup-remove.sh | 69 +++++++++++++++++++++++++++++++++++++
 tests/local.mk            |  1 +
 4 files changed, 157 insertions(+), 3 deletions(-)
 create mode 100755 tests/df/df-dup-remove.sh

diff --git a/NEWS b/NEWS
index d4aebeb..d0e13f6 100644
--- a/NEWS
+++ b/NEWS
@@ -22,6 +22,9 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   another range.  Before, "echo 123|cut --output-delim=: -b2-,3" would print
   "2:3".  Now it prints "23".  [bug introduced in 5.3.0]
 
+  df now properly outputs filesystem information with bind mounts
+  present on the system.
+
   install -m M SOURCE DEST no longer has a race condition where DEST's
   permissions are temporarily derived from SOURCE instead of from M.
 
diff --git a/src/df.c b/src/df.c
index cac26b7..bc3310d 100644
--- a/src/df.c
+++ b/src/df.c
@@ -43,6 +43,17 @@
   proper_name ("David MacKenzie"), \
   proper_name ("Paul Eggert")
 
+/* Filled with device numbers of examined filesystems to avoid
+   duplicities in output.  */
+struct devlist
+{
+  dev_t dev_num;
+  struct devlist *dev_next;
+};
+
+/* Device list help pointer.  */
+static struct devlist *devlist_head;
+
 /* If true, show even file systems with zero size or
    uninteresting types.  */
 static bool show_all_fs;
@@ -54,6 +65,9 @@ static bool show_local_fs;
    command line argument -- even if it's a dummy (automounter) entry.  */
 static bool show_listed_fs;
 
+/* If true, include rootfs in the output.  */
+static bool show_rootfs;
+
 /* Human-readable options for output.  */
 static int human_output_opts;
 
@@ -1076,6 +1090,50 @@ get_entry (char const *name, struct stat const *statp)
   get_point (name, statp);
 }
 
+/* Add a device number of the soon-to-be examined
+   filesystem to the global list devlist.  */
+
+static void
+devlist_add (dev_t dev_num)
+{
+  struct devlist *devlist = devlist_head;
+  while (devlist->dev_next)
+    devlist = devlist->dev_next;
+
+  if (!devlist->dev_num)
+      devlist->dev_num = dev_num;
+  else
+    {
+      devlist->dev_next = xmalloc (sizeof *devlist);
+      devlist = devlist->dev_next;
+      devlist->dev_num = dev_num;
+      devlist->dev_next = NULL;
+    }
+}
+
+/* Check if the device was already examined.  */
+
+static bool
+dev_examined (struct devlist *devlist, char *mount_dir, char *devname)
+{
+  dev_t dev_num;
+  struct stat buf;
+  stat (mount_dir, &buf);
+  if (!(show_rootfs || STRNCMP_LIT (devname, "rootfs")))
+    return true;
+
+  dev_num = buf.st_dev;
+  while (devlist)
+    {
+      if (devlist->dev_num == dev_num)
+        return true;
+      devlist = devlist->dev_next;
+    }
+
+  devlist_add (dev_num);
+  return false;
+}
+
 /* Show all mounted file systems, except perhaps those that are of
    an unselected type or are empty.  */
 
@@ -1084,9 +1142,31 @@ 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 (show_all_fs)
+    {
+      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);
+    }
+  else
+    {
+      struct devlist *devlist = xmalloc (sizeof *devlist);
+      devlist_head = devlist;
+      devlist->dev_num = 0;
+      devlist->dev_next = NULL;
+
+      for (me = mount_list; me; me = me->me_next)
+        if (!dev_examined (devlist, me->me_mountdir, me->me_devname))
+          get_dev (me->me_devname, me->me_mountdir, NULL, me->me_type,
+                   me->me_dummy, me->me_remote, NULL, true);
+
+      while (devlist)
+        {
+          devlist = devlist->dev_next;
+          free (devlist_head);
+          devlist_head = devlist;
+        }
+    }
 }
 
 /* Add FSTYPE to the list of file system types to display.  */
@@ -1283,6 +1363,7 @@ main (int argc, char **argv)
           /* Accept -F as a synonym for -t for compatibility with Solaris.  */
         case 't':
           add_fs_type (optarg);
+          show_rootfs = selected_fstype ("rootfs");
           break;
 
         case 'v':              /* For SysV compatibility.  */
diff --git a/tests/df/df-dup-remove.sh b/tests/df/df-dup-remove.sh
new file mode 100755
index 0000000..d097d4a
--- /dev/null
+++ b/tests/df/df-dup-remove.sh
@@ -0,0 +1,69 @@
+#!/bin/sh
+# Test df's behaviour when the mount list contains duplicit entries.
+# This test is skipped on systems that lack LD_PRELOAD support; that's fine.
+
+# Copyright (C) 2012 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
+print_ver_ df
+
+df || skip_ "df fails"
+
+# Simulate "mtab" failure.
+cat > k.c <<'EOF' || framework_failure_
+#include <stdio.h>
+#include <mntent.h>
+
+struct mntent *getmntent (FILE *fp)
+{
+  /* Prove that LD_PRELOAD works. */
+  static int done = 0;
+  if (!done)
+    {
+      fclose (fopen ("x", "w"));
+      ++done;
+    }
+  /* Now simulate an mtab file with
+  two entries.  */
+
+  static struct mntent mntent;
+
+  while (done++ < 3)
+    {
+      mntent.mnt_fsname = "fsname";
+      mntent.mnt_dir = "/";
+      mntent.mnt_type = "-";
+
+      return &mntent;
+    }
+  return NULL;
+}
+EOF
+
+# Then compile/link it:
+gcc --std=gnu99 -shared -fPIC -ldl -O2 k.c -o k.so \
+  || skip_ "getmntent hack does not work on this platform"
+
+# Test if LD_PRELOAD works:
+LD_PRELOAD=./k.so df
+test -f x || skip_ "internal test failure: maybe LD_PRELOAD doesn't work?"
+
+#The fake mtab file should only contain 2 entries, both
+#having the same device number; thus the output should
+#consist of a header and one entry
+test $(LD_PRELOAD=./k.so df | wc -l) -eq 2 || fail=1
+
+Exit $fail
diff --git a/tests/local.mk b/tests/local.mk
index 1b0ace4..8f5def2 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -457,6 +457,7 @@ all_tests =                                 \
   tests/df/unreadable.sh                       \
   tests/df/total-unprocessed.sh                        \
   tests/df/no-mtab-status.sh                   \
+  tests/df/df-dup-remove.sh                    \
   tests/dd/direct.sh                           \
   tests/dd/misc.sh                             \
   tests/dd/nocache.sh                          \
-- 
1.7.11.7

Reply via email to