Re: [PATCH] remove "dev=" mount option special processing for df

[Paul Eggert]

"Paul A. Clarke" <address@hidden> writes:

> Why keep around and support obscure (obsolete?) code

Obscure, yes.  Obsolete, no.  The code is used in some non-Linux
kernels, and it improves performance greatly in some cases.  The idea
is that you don't want to stat all the mounted file systems if you can
avoid it.  E.g., "df ." where /bin/pwd would fail and there are lots
of NFS file systems -- in that case the code can make the difference
between outputting the correct answer right away and hanging
indefinitely.

Come to think of it, the idea could be used even in the latest Solaris
release too.  The patch below incorporates this.

> 1.  Deprecate and/or remove the support for "dev=" parsing, if not
> everywhere then at least where it's not supported by the filesystem, or
> minimally (selfishly) just for Linux.

OK, you've convinced me for GNU/Linux anyway.

How about this patch?  I haven't installed it (the code is currently
frozen) but I'd like you to test it if possible.


2005-10-21  Paul Eggert  <address@hidden>

        * doc/coreutils.texi (df invocation): Correct/modernize documentation
        for which file systems are ignored by default.

        * lib/mountlist.c: Make the parsing of "dev=..." more robust.
        Include <limits.h>.
        (dev_from_mount_options) [defined MOUNTED_GETMNTENT1 || defined
        MOUNTED_GETMNTENT2]: New function, with more-robust checking.
        Do not look for dev= on GNU/Linux; problem reported by Paul A. Clarke.
        (read_file_system_list) [defined MOUNTED_GETMNTENT1 || defined
        MOUNTED_GETMNTENT2]: Use it.  This improves performance in the
        MOUNTED_GETMNTENT2 case, when /bin/pwd would fail.

        * lib/mountlist.c (ME_DUMMY) [!defined ME_DUMMY]: Ignore "none"
        file systems.  Problem reported by Bob Proulx.
        Ignore "proc" file systems.  Problem reported by address@hidden

Index: doc/coreutils.texi
===================================================================
RCS file: /fetish/cu/doc/coreutils.texi,v
retrieving revision 1.289
diff -p -u -r1.289 coreutils.texi
--- doc/coreutils.texi  16 Oct 2005 07:27:04 -0000      1.289
+++ doc/coreutils.texi  21 Oct 2005 21:53:27 -0000
@@ -8722,11 +8722,9 @@ The program accepts the following option
 @opindex --all
 @cindex automounter file systems
 @cindex ignore file systems
-Include in the listing file systems that have a size of 0 blocks, which
+Include in the listing dummy file systems, which
 are omitted by default.  Such file systems are typically special-purpose
-pseudo-file-systems, such as automounter entries.  Also, file systems of
-type ``ignore'' or ``auto'', supported by some operating systems, are
-only included if this option is specified.
+pseudo-file-systems, such as automounter entries.
 
 @item -B @var{size}
 @itemx address@hidden
Index: lib/mountlist.c
===================================================================
RCS file: /fetish/cu/lib/mountlist.c,v
retrieving revision 1.55
diff -p -u -r1.55 mountlist.c
--- lib/mountlist.c     22 Sep 2005 06:05:39 -0000      1.55
+++ lib/mountlist.c     21 Oct 2005 21:53:28 -0000
@@ -23,6 +23,7 @@
 
 #include "mountlist.h"
 
+#include <limits.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -143,6 +144,8 @@ char *strstr ();
 #ifndef ME_DUMMY
 # define ME_DUMMY(Fs_name, Fs_type)            \
     (strcmp (Fs_type, "autofs") == 0           \
+     || strcmp (Fs_type, "none") == 0          \
+     || strcmp (Fs_type, "proc") == 0          \
      || strcmp (Fs_type, "subfs") == 0         \
      /* for Irix 6.5 */                                \
      || strcmp (Fs_type, "ignore") == 0)
@@ -283,6 +286,44 @@ fstype_to_string (int t)
 }
 #endif /* MOUNTED_VMOUNT */
 
+
+#if defined MOUNTED_GETMNTENT1 || defined MOUNTED_GETMNTENT2
+
+/* Return the device number from MOUNT_OPTIONS, if possible.
+   Otherwise return (dev_t) -1.  */
+
+static dev_t
+dev_from_mount_options (char const *mount_options)
+{
+  /* GNU/Linux allows file system implementations to define their own
+     meaning for "dev=" mount options, so don't trust the meaning
+     here.  */
+#ifndef __linux__
+
+  static char const dev_pattern[] = ",dev=";
+  char const *devopt = strstr (mount_options, dev_pattern);
+
+  if (devopt)
+    {
+      char const *optval = devopt + sizeof dev_pattern - 1;
+      char *optvalend;
+      unsigned long int dev;
+      errno = 0;
+      dev = strtoul (optval, &optvalend, 16);
+      if (optval != optvalend
+         && (*optvalend == '\0' || *optvalend == ',')
+         && ! (dev == ULONG_MAX && errno == ERANGE)
+         && dev == (dev_t) dev)
+       return dev;
+    }
+
+#endif
+
+  return -1;
+}
+
+#endif
+
 /* Return a list of the currently mounted file systems, or NULL on error.
    Add each entry to the tail of the list so that they stay in order.
    If NEED_FS_TYPE is true, ensure that the file system type fields in
@@ -325,12 +366,11 @@ read_file_system_list (bool need_fs_type
   }
 #endif
 
-#ifdef MOUNTED_GETMNTENT1      /* 4.3BSD, SunOS, HP-UX, Dynix, Irix.  */
+#ifdef MOUNTED_GETMNTENT1 /* GNU/Linux, 4.3BSD, SunOS, HP-UX, Dynix, Irix.  */
   {
     struct mntent *mnt;
     char *table = MOUNTED;
     FILE *fp;
-    char *devopt;
 
     fp = setmntent (table, "r");
     if (fp == NULL)
@@ -345,11 +385,7 @@ read_file_system_list (bool need_fs_type
        me->me_type_malloced = 1;
        me->me_dummy = ME_DUMMY (me->me_devname, me->me_type);
        me->me_remote = ME_REMOTE (me->me_devname, me->me_type);
-       devopt = strstr (mnt->mnt_opts, "dev=");
-       if (devopt)
-         me->me_dev = strtoul (devopt + 4, NULL, 16);
-       else
-         me->me_dev = (dev_t) -1;      /* Magic; means not known yet. */
+       me->me_dev = dev_from_mount_options (mnt->mnt_opts);
 
        /* Add to the linked list. */
        *mtail = me;
@@ -623,7 +659,7 @@ read_file_system_list (bool need_fs_type
   }
 #endif /* MOUNTED_FREAD || MOUNTED_FREAD_FSTYP.  */
 
-#ifdef MOUNTED_GETMNTTBL       /* DolphinOS goes it's own way */
+#ifdef MOUNTED_GETMNTTBL       /* DolphinOS goes its own way.  */
   {
     struct mntent **mnttbl = getmnttbl (), **ent;
     for (ent=mnttbl;*ent;ent++)
@@ -697,7 +733,7 @@ read_file_system_list (bool need_fs_type
            me->me_type_malloced = 1;
            me->me_dummy = MNT_IGNORE (&mnt) != 0;
            me->me_remote = ME_REMOTE (me->me_devname, me->me_type);
-           me->me_dev = (dev_t) -1;    /* Magic; means not known yet. */
+           me->me_dev = dev_from_mount_options (mnt.mnt_mntopts);
 
            /* Add to the linked list. */
            *mtail = me;