Re: [PATCH] df: fail when the mount list cannot be read but is needed [was: new snapshot available: coreutils-8.17.70-1bacb4]

[Jim Meyering]

Bernhard Voelker wrote:
> On 08/09/2012 01:53 PM, Bernhard Voelker wrote:
>> Here's a proposed patch.
>
> Oops, sorry, one of the cases in the new test failed.
> I fixed it:
>
> -LD_PRELOAD=./k.so df --total || fail=1
> +LD_PRELOAD=./k.so df --total && fail=1
>
> Additionally, as this patch didn't make it into 8.18,
> I moved the NEWS entry into the new release.

Thanks.  It came a little too late for 8.18.

Here is the patch that I expect to squash in.
Most are nits, but there was an exploitable bit
of (debugging?) code in the new test.

diff --git a/NEWS b/NEWS
index 0f63ddf..46d0a41 100644
--- a/NEWS
+++ b/NEWS
@@ -2,8 +2,8 @@ GNU coreutils NEWS                                    -*- 
outline -*-

 * Noteworthy changes in release ?.? (????-??-??) [?]

-  df now fails in cases when the list of mounted file systems (/etc/mtab)
-  cannot be read but the file system type information is needed to process
+  df now fails when the list of mounted file systems (/etc/mtab) cannot
+  be read, yet the file system type information is needed to process
   certain options like -a, -l, -t and -x.
   [This bug was present in "the beginning".]

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 2fe0dd3..744b41a 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -10754,9 +10754,9 @@ df invocation
 @var{dir}} to test whether @var{dir} is on a file system of type
 @samp{ext3} or @samp{reiserfs}.

-As the list of file systems (@var{mtab}) is needed to determine the
+Since the list of file systems (@var{mtab}) is needed to determine the
 file system type, failure includes the cases when that list cannot
-be read and either of the options @option{-a}, @option{-l}, @option{-t}
+be read and one or more of the options @option{-a}, @option{-l}, @option{-t}
 or @option{-x} is used together with a file name argument.


diff --git a/src/df.c b/src/df.c
index a2e6866..835cc2e 100644
--- a/src/df.c
+++ b/src/df.c
@@ -1104,12 +1104,12 @@ main (int argc, char **argv)
          or when either of -a, -l, -t or -x is used with file name
          arguments. Otherwise, merely give a warning and proceed.  */
       int status = 0;
-      if ( !(optind < argc)
-          ||(optind < argc
-             && (show_all_fs
-                 || show_local_fs
-                 || fs_select_list != NULL
-                 || fs_exclude_list != NULL)))
+      if ( ! (optind < argc)
+           || (optind < argc
+               && (show_all_fs
+                   || show_local_fs
+                   || fs_select_list != NULL
+                   || fs_exclude_list != NULL)))
         {
           status = EXIT_FAILURE;
         }
diff --git a/tests/df/no-mtab-status b/tests/df/no-mtab-status
index 8835345..6e07854 100755
--- a/tests/df/no-mtab-status
+++ b/tests/df/no-mtab-status
@@ -27,7 +27,7 @@ cat > k.c <<'EOF' || framework_failure_
 #include <stdio.h>
 #include <errno.h>

-struct mntent *getmntent(FILE *fp)
+struct mntent *getmntent (FILE *fp)
 {
   /* Prove that LD_PRELOAD works. */
   static int done = 0;
@@ -46,8 +46,6 @@ EOF
 $CC -shared -fPIC -ldl -O2 k.c -o k.so \
   || framework_failure_ 'failed to compile with -shared -fPIC'

-cp k.so /tmp
-
 # Test if LD_PRELOAD works:
 LD_PRELOAD=./k.so df
 test -f x || skip_ "internal test failure: maybe LD_PRELOAD doesn't work?"