>From 62112df539326d9f874ad44a94f70b2a9749a975 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= Date: Sat, 25 Jan 2014 01:14:29 +0000 Subject: [PATCH 1/4] df: also deduplicate virtual file systems * src/df.c (filter_mountlist): Remove the constraint that a '/' needs to be in the device name for a mount entry to be considered for deduplication. Virtual file systems also have storage associated with them (like tmpfs for example), and thus need to be deduplicated since they will be shown in the default df output and subject to --total processing also. * test/df/skip-duplicates.sh: Add a test to ensure we deduplicate all entries, even for virtual file systems. Also avoid possible length operations on many remote file systems in the initial check of df operation. Also avoid the assumption that "/root" is on the same file system as "/". * NEWS: Mention the change in behavior. --- NEWS | 2 + src/df.c | 31 ++++++++++++--------------- tests/df/skip-duplicates.sh | 47 +++++++++++++++++++++++++++++++----------- 3 files changed, 50 insertions(+), 30 deletions(-) diff --git a/NEWS b/NEWS index 4efd60d..c204b68 100644 --- a/NEWS +++ b/NEWS @@ -44,6 +44,8 @@ GNU coreutils NEWS -*- outline -*- [These dd bugs were present in "the beginning".] + df now correctly elides duplicates for virtual file systems like tmpfs. + head --bytes=-N and --lines=-N now handles devices more consistently, not ignoring data from virtual devices like /dev/zero, or on BSD systems data from tty devices. diff --git a/src/df.c b/src/df.c index e763943..2b5a54e 100644 --- a/src/df.c +++ b/src/df.c @@ -630,26 +630,23 @@ filter_mount_list (void) } else { - /* If the device name is a real path name ... */ - if (strchr (me->me_devname, '/')) + /* If we've already seen this device... */ + for (devlist = devlist_head; devlist; devlist = devlist->next) + if (devlist->dev_num == buf.st_dev) + break; + + if (devlist) { - /* ... try to find its device number in the devlist. */ - for (devlist = devlist_head; devlist; devlist = devlist->next) - if (devlist->dev_num == buf.st_dev) - break; + discard_me = me; - 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))) { - discard_me = me; - - /* Let the shorter mountdir win. */ - if (! strchr (devlist->me->me_devname, '/') - || (strlen (devlist->me->me_mountdir) - > strlen (me->me_mountdir))) - { - discard_me = devlist->me; - devlist->me = me; - } + discard_me = devlist->me; + devlist->me = me; } } } diff --git a/tests/df/skip-duplicates.sh b/tests/df/skip-duplicates.sh index 266520a..d872f27 100755 --- a/tests/df/skip-duplicates.sh +++ b/tests/df/skip-duplicates.sh @@ -21,19 +21,28 @@ print_ver_ df require_gcc_shared_ -df || skip_ "df fails" +# We use --local here so as to not activate +# potentially very many remote mounts. +df --local || skip_ "df fails" -# Simulate an mtab file with two entries of the same device number. -# Also add entries with unstatable mount dirs to ensure that's handled. +export CU_NONROOT_FS=$(df --local --output=target 2>&1 | grep /. | head -n1) +test -z "$CU_NONROOT_FS" && unique_entries=1 || unique_entries=2 + +# Simulate an mtab file to test various cases. cat > k.c <<'EOF' || framework_failure_ #include #include +#include #include +#define STREQ(a, b) (strcmp (a, b) == 0) + struct mntent *getmntent (FILE *fp) { + static char *nonroot_fs; + static int done; + /* Prove that LD_PRELOAD works. */ - static int done = 0; if (!done) { fclose (fopen ("x", "w")); @@ -43,18 +52,30 @@ struct mntent *getmntent (FILE *fp) static struct mntent mntents[] = { {.mnt_fsname="/short", .mnt_dir="/invalid/mount/dir"}, {.mnt_fsname="fsname", .mnt_dir="/",}, - {.mnt_fsname="/fsname", .mnt_dir="/root"}, + {.mnt_fsname="/fsname", .mnt_dir="/."}, {.mnt_fsname="/fsname", .mnt_dir="/"}, + {.mnt_fsname="virtfs", .mnt_dir="/NONROOT"}, + {.mnt_fsname="virtfs", .mnt_dir="/NONROOT"}, }; - if (!getenv ("CU_TEST_DUPE_INVALID") && done == 1) + if (done == 1) + { + nonroot_fs = getenv ("CU_NONROOT_FS"); + if (!nonroot_fs || !*nonroot_fs) + nonroot_fs = "/"; /* merge into / entries. */ + } + + if (done == 1 && !getenv ("CU_TEST_DUPE_INVALID")) done++; /* skip the first entry. */ - while (done++ <= 4) + while (done++ <= 6) { mntents[done-2].mnt_type = "-"; + if (STREQ (mntents[done-2].mnt_dir, "/NONROOT")) + mntents[done-2].mnt_dir = nonroot_fs; return &mntents[done-2]; } + return NULL; } EOF @@ -69,22 +90,22 @@ test -f x || skip_ "internal test failure: maybe LD_PRELOAD doesn't work?" # The fake mtab file should only contain entries # having the same device number; thus the output should -# consist of a header and one entry. +# consist of a header and unique entries. LD_PRELOAD=./k.so df >out || fail=1 -test $(wc -l out && fail=1 -test $(wc -l out || fail=1 -test $(wc -l From 3449cad1576a3d584b248524bd03ed1b3d6bcb0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= Date: Mon, 12 May 2014 13:29:01 +0100 Subject: [PATCH 2/4] df: fix handling of symlinks in mount list The symlink handling in commit v8.21-172-g33660b4 was incomplete in the case where there were symlinks in the mount list itself. For example, in the case where /dev/mapper/fedora-home was in the mount list and that in turn was a symlink to /dev/dm-2, we have: before> df --out=source /dev/mapper/fedora-home devtmpfs after > df --out=source /dev/mapper/fedora-home /dev/mapper/fedora-home * src/df.c (get_disk): Compare canonicalized device names from the mount list. Note we still display the non canonicalized name, even if longer, as we assume that is the most representative. * tests/df/df-symlink.sh: This could theoretically fail on some systems depending on the content of the mount list, but adjust to fail on any system where symlinks are present in the mount list for the current dir. --- src/df.c | 12 ++++++++++-- tests/df/df-symlink.sh | 7 +++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/df.c b/src/df.c index 2b5a54e..24897a3 100644 --- a/src/df.c +++ b/src/df.c @@ -1056,13 +1056,19 @@ get_disk (char const *disk) char const *file = disk; char *resolved = canonicalize_file_name (disk); - if (resolved && resolved[0] == '/') + if (resolved && IS_ABSOLUTE_FILE_NAME (resolved)) disk = resolved; size_t best_match_len = SIZE_MAX; for (me = mount_list; me; me = me->me_next) { - if (STREQ (disk, me->me_devname)) + /* TODO: Should cache canon_dev in the mount_entry struct. */ + char *devname = me->me_devname; + char *canon_dev = canonicalize_file_name (me->me_devname); + if (canon_dev && IS_ABSOLUTE_FILE_NAME (canon_dev)) + devname = canon_dev; + + if (STREQ (disk, devname)) { size_t len = strlen (me->me_mountdir); if (len < best_match_len) @@ -1074,6 +1080,8 @@ get_disk (char const *disk) best_match_len = len; } } + + free (canon_dev); } free (resolved); diff --git a/tests/df/df-symlink.sh b/tests/df/df-symlink.sh index aaed810..6d96bd2 100755 --- a/tests/df/df-symlink.sh +++ b/tests/df/df-symlink.sh @@ -28,4 +28,11 @@ df --out=source,target "$disk" > exp || skip_ "cannot get info for $disk" df --out=source,target symlink > out || fail=1 compare exp out || fail=1 +# Ensure we output the same values for device nodes and '.' +# This was not the case in coreutil-8.22 on systems +# where the device in the mount list was a symlink itself. +# I.E. '.' => /dev/mapper/fedora-home -> /dev/dm-2 +df --out=source,target '.' > out || fail=1 +compare exp out || fail=1 + Exit $fail -- 1.7.7.6 >From 3e6da84f0af81b2a8a9539b14889f299680d7f7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= Date: Mon, 12 May 2014 14:49:13 +0100 Subject: [PATCH 3/4] df: ignore non file system entries in /proc/mounts Linux with network namespaces contains entries in /proc/mounts like: proc net:[4026532464] proc rw,nosuid,nodev,noexec,relatime 0 0 resulting in a failure to stat 'net:[...]', inducing a warning and an exit with failure status. * src/df.c (get_dev): Ignore all relative mount points. * tests/df/skip-duplicates.sh: Add an entry to test relative dirs. --- src/df.c | 5 +++++ tests/df/skip-duplicates.sh | 3 ++- 2 files changed, 7 insertions(+), 1 deletions(-) diff --git a/src/df.c b/src/df.c index 24897a3..a7fc57f 100644 --- a/src/df.c +++ b/src/df.c @@ -853,6 +853,11 @@ get_dev (char const *disk, char const *mount_point, char const* file, if (!selected_fstype (fstype) || excluded_fstype (fstype)) return; + /* Ignore relative MOUNT_POINTs, which are present for example + in /proc/mounts on Linux with network namespaces. */ + if (!force_fsu && mount_point && ! IS_ABSOLUTE_FILE_NAME (mount_point)) + return; + /* If MOUNT_POINT is NULL, then the file system is not mounted, and this program reports on the file system that the special file is on. It would be better to report on the unmounted file system, diff --git a/tests/df/skip-duplicates.sh b/tests/df/skip-duplicates.sh index d872f27..44f7d4c 100755 --- a/tests/df/skip-duplicates.sh +++ b/tests/df/skip-duplicates.sh @@ -56,6 +56,7 @@ struct mntent *getmntent (FILE *fp) {.mnt_fsname="/fsname", .mnt_dir="/"}, {.mnt_fsname="virtfs", .mnt_dir="/NONROOT"}, {.mnt_fsname="virtfs", .mnt_dir="/NONROOT"}, + {.mnt_fsname="netns", .mnt_dir="net:[1234567]"}, }; if (done == 1) @@ -68,7 +69,7 @@ struct mntent *getmntent (FILE *fp) if (done == 1 && !getenv ("CU_TEST_DUPE_INVALID")) done++; /* skip the first entry. */ - while (done++ <= 6) + while (done++ <= 7) { mntents[done-2].mnt_type = "-"; if (STREQ (mntents[done-2].mnt_dir, "/NONROOT")) -- 1.7.7.6 >From 6e4f211d62110a719a42b13da59e5c9b073f29fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= Date: Mon, 12 May 2014 15:46:43 +0100 Subject: [PATCH 4/4] maint: avoid clang -Wtautological-constant-out-of-range-compare warning * src/df.c (decode_output_arg): Use only enum constants to avoid clang "warning: comparison of constant -1 with expression of type 'display_field_t' is always false" --- src/df.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/df.c b/src/df.c index a7fc57f..01ecca6 100644 --- a/src/df.c +++ b/src/df.c @@ -144,7 +144,8 @@ typedef enum IAVAIL_FIELD, /* inodes available */ IPCENT_FIELD, /* inodes used in percent */ TARGET_FIELD, /* mount point */ - FILE_FIELD /* specified file name */ + FILE_FIELD, /* specified file name */ + INVALID_FIELD /* validation marker */ } display_field_t; /* Flag if a field contains a block, an inode or another value. */ @@ -372,7 +373,7 @@ decode_output_arg (char const *arg) *comma++ = 0; /* process S. */ - display_field_t field = -1; + display_field_t field = INVALID_FIELD; for (unsigned int i = 0; i < ARRAY_CARDINALITY (field_data); i++) { if (STREQ (field_data[i].arg, s)) @@ -381,7 +382,7 @@ decode_output_arg (char const *arg) break; } } - if (field == -1) + if (field == INVALID_FIELD) { error (0, 0, _("option --output: field %s unknown"), quote (s)); usage (EXIT_FAILURE); -- 1.7.7.6