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

[Bernhard Voelker]

On 08/08/2012 06:14 PM, Pádraig Brady wrote:
> But then it won't fail. It's tricky :)

I thought about that case again, and to me something's still wrong
when mtab is missing.

When mtab cannot be read, then df cannot determine the file system type.

Example (/tmp/mnt is an nfs mount):

  $ src/df -T  /tmp/mnt ; echo $?
  src/df: Warning: cannot read table of mounted file systems: No such file or 
directory
  Filesystem     Type 1K-blocks      Used Available Use% Mounted on
  -              -    155527168 114567680  33058816  78% /tmp/mnt
  0

I think $? == 0 is okay for this case.

  $ src/df -t hello /tmp/mnt ; echo $?
  src/df: Warning: cannot read table of mounted file systems: No such file or 
directory
  Filesystem     1K-blocks      Used Available Use% Mounted on
  -              155527168 114567680  33058816  78% /tmp/mnt
  0

  $ src/df -x nfs /tmp/mnt ; echo $?
  src/df: Warning: cannot read table of mounted file systems: No such file or 
directory
  Filesystem     1K-blocks      Used Available Use% Mounted on
  -              155527168 114567680  33058816  78% /tmp/mnt
  0

But in the case -t/-x I believe that df should exit(1).
The same applies to -a and -l.

Here's a proposed patch.

Have a nice day,
Berny

>From 32e3007a8cab32484e4e312ceb1784f6ad1d396f Mon Sep 17 00:00:00 2001
From: Bernhard Voelker <address@hidden>
Date: Thu, 9 Aug 2012 13:46:30 +0200
Subject: [PATCH] df: fail when the mount list cannot be read but is needed

* src/df.c (main): Add conditions to fail when the mount list cannot
be read: this includes the cases when a file name argument is given
and either of -a, -l -t or -x is used.
* doc/coreutils.texi: Document the additional error conditions.
* tests/df/no-mtab-status: Add a new test.
* tests/Makefile.am: Reference the new test.
* NEWS: Mention the fix.
---
 NEWS                    |    5 +++
 doc/coreutils.texi      |    5 +++
 src/df.c                |   18 ++++++++--
 tests/Makefile.am       |    1 +
 tests/df/no-mtab-status |   81 +++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 106 insertions(+), 4 deletions(-)
 create mode 100755 tests/df/no-mtab-status

diff --git a/NEWS b/NEWS
index ca4568a..d0d462c 100644
--- a/NEWS
+++ b/NEWS
@@ -22,6 +22,11 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   file system --type options do not lead to a processed file system.
   [This bug dates back to when --total was added in coreutils-7.0]

+  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
+  certain options like -a, -l, -t and -x.
+  [This bug was present in "the beginning".]
+
   head --lines=-N (-n-N) now resets the read pointer of a seekable input file.
   This means that "head -n-3" no longer consumes all of its input, and lines
   not output by head may be processed by other programs.  For example, this
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 516ec73..2fe0dd3 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -10754,6 +10754,11 @@ inspect the exit status of a command like @samp{df -t 
ext3 -t reiserfs
 @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
+file system type, failure includes the cases when that list cannot
+be read and either of the options @option{-a}, @option{-l}, @option{-t}
+or @option{-x} is used together with a file name argument.
+

 @node du invocation
 @section @command{du}: Estimate file space usage
diff --git a/src/df.c b/src/df.c
index 4f3b56b..a2e6866 100644
--- a/src/df.c
+++ b/src/df.c
@@ -1100,10 +1100,20 @@ main (int argc, char **argv)
   if (mount_list == NULL)
     {
       /* Couldn't read the table of mounted file systems.
-         Fail if df was invoked with no file name arguments;
-         Otherwise, merely give a warning and proceed.  */
-      int status =          (optind < argc ? 0 : EXIT_FAILURE);
-      const char *warning = (optind < argc ? _("Warning: ") : "");
+         Fail if df was invoked with no file name arguments,
+         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)))
+        {
+          status = EXIT_FAILURE;
+        }
+      const char *warning = (status == 0 ? _("Warning: ") : "");
       error (status, errno, "%s%s", warning,
              _("cannot read table of mounted file systems"));
     }
diff --git a/tests/Makefile.am b/tests/Makefile.am
index edc04b4..273405f 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -376,6 +376,7 @@ TESTS =                                             \
   df/df-P                                      \
   df/unreadable                                        \
   df/total-unprocessed                         \
+  df/no-mtab-status                            \
   dd/direct                                    \
   dd/misc                                      \
   dd/nocache                                   \
diff --git a/tests/df/no-mtab-status b/tests/df/no-mtab-status
new file mode 100755
index 0000000..f568842
--- /dev/null
+++ b/tests/df/no-mtab-status
@@ -0,0 +1,81 @@
+#!/bin/sh
+# Test df's behaviour when the mount list cannot be read.
+# 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=.}/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 <errno.h>
+
+struct mntent *getmntent(FILE *fp)
+{
+  /* Prove that LD_PRELOAD works. */
+  static int done = 0;
+  if (!done)
+    {
+      fclose (fopen ("x", "w"));
+      ++done;
+    }
+  /* Now simulate the failure. */
+  errno = ENOENT;
+  return NULL;
+}
+EOF
+
+# Then compile/link it:
+$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?"
+
+# These tests are supposed to succeed:
+LD_PRELOAD=./k.so df '.' || fail=1
+LD_PRELOAD=./k.so df -i '.' || fail=1
+LD_PRELOAD=./k.so df -T '.' || fail=1
+LD_PRELOAD=./k.so df -Ti '.' || fail=1
+LD_PRELOAD=./k.so df --total '.' || fail=1
+
+# These tests are supposed to fail:
+LD_PRELOAD=./k.so df && fail=1
+LD_PRELOAD=./k.so df -i && fail=1
+LD_PRELOAD=./k.so df -T && fail=1
+LD_PRELOAD=./k.so df -Ti && fail=1
+LD_PRELOAD=./k.so df --total || fail=1
+
+LD_PRELOAD=./k.so df -a && fail=1
+LD_PRELOAD=./k.so df -a '.' && fail=1
+
+LD_PRELOAD=./k.so df -l && fail=1
+LD_PRELOAD=./k.so df -l '.' && fail=1
+
+LD_PRELOAD=./k.so df -t hello && fail=1
+LD_PRELOAD=./k.so df -t hello '.' && fail=1
+
+LD_PRELOAD=./k.so df -x hello && fail=1
+LD_PRELOAD=./k.so df -x hello '.' && fail=1
+
+Exit $fail
-- 
1.7.7