Re: proposed pathchk change, in response to today's POSIX interpretation

[Paul Eggert]

Jim Meyering <address@hidden> writes:

> I know it's a pain, but would you please add a few tests to
> go with these changes?

OK, done.  Also, I fixed the code, documentation, and tests to align
with another POSIX interpretation issued the same day, which I missed,
which said that "pathchk -P ''" should fail.  Here's what I installed.
(Sorry, I'm not sure what header you wanted in the NEWS file for post-5.3.0
changes so I made something up):

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

        Respond to POSIX interpretations about pathchk -p dated 2005-01-06.
        * NEWS: Document the changes.
        * doc/coreutils.texi (pathchk invocation): Likewise.
        * src/pathchk.c (PORTABILITY_OPTION): New constant.
        (longopts, usage, main, validate_file_name):
        Add support for new -P option.
        Reject empty file names (unless -p is not specified and the
        current system allows empty file names).
        Change --portability so that is now equivalent to -p -P.
        Don't test whether file name is too long, if it is known to exist.
        (no_leading_hyphen): New function.
        * tests/misc/pathchk1: Add tests for empty file names and
        pathchk -P.

Index: NEWS
===================================================================
RCS file: /fetish/cu/NEWS,v
retrieving revision 1.264
diff -p -u -r1.264 NEWS
--- NEWS        9 Jan 2005 19:45:00 -0000       1.264
+++ NEWS        10 Jan 2005 18:05:57 -0000
@@ -1,5 +1,21 @@
 GNU coreutils NEWS                                    -*- outline -*-
 
+* Major changes in release 5.3.1 (2005-??-??) [unstable]
+
+  pathchk changes:
+
+    It now rejects the empty name in the normal case.  That is,
+    "pathchk -p ''" now fails, and "pathchk ''" fails unless the
+    current host (contra POSIX) allows empty file names.
+
+    The new -P option checks whether a file name component has leading "-",
+    as suggested in interpretation "Austin-039:XCU:pathchk:pathchk -p"
+    <http://www.opengroup.org/austin/interps/doc.tpl?gdid=6232>.
+    It also rejects the empty name even if the current host accepts it; see
+    <http://www.opengroup.org/austin/interps/doc.tpl?gdid=6233>.
+
+    The --portability option is now equivalent to -p -P.
+
 * Major changes in release 5.3.0 (2005-01-08) [unstable]
 
 ** Bug fixes
Index: doc/coreutils.texi
===================================================================
RCS file: /fetish/cu/doc/coreutils.texi,v
retrieving revision 1.237
diff -p -u -r1.237 coreutils.texi
--- doc/coreutils.texi  7 Jan 2005 20:10:15 -0000       1.237
+++ doc/coreutils.texi  10 Jan 2005 18:05:59 -0000
@@ -10240,21 +10240,39 @@ its file system's maximum.
 A nonexistent @var{name} is not an error, so long a file with that
 name could be created under the above conditions.
 
-The program accepts the following option.  Also see @ref{Common options}.
+The program accepts the following options.  Also see @ref{Common options}.
 Options must precede operands.
 
 @table @samp
 
 @item -p
address@hidden --portability
 @opindex -p
address@hidden --portability
-Do not perform checks based on the underlying file system.  Instead,
-check the length of each file name and its components against the
address@hidden minimum limits for portability.  Also check that the file
-name contains only characters that are in the portable file name
+Instead of performing checks based on the underlying file system,
+print a message if any of these conditions is true:
+
address@hidden
address@hidden
+A file name is empty.
+
address@hidden
+The length of a file name or one of its components exceeds the
address@hidden minimum limits for portability.
+
address@hidden
+A file name contains a character outside the portable file name
 character set, namely, the ASCII letters and digits, @samp{-},
 @samp{.}, @samp{/}, and @samp{_}.
address@hidden enumerate
+
address@hidden -P
address@hidden -P
+Print a message if a file name is empty, or if it contains a component
+that begins with @samp{-}.
+
address@hidden --portability
address@hidden --portability
+Print a message if a file name is not portable to all @acronym{POSIX}
+hosts.  This option is equivalent to @samp{-p -P}.
 
 @end table
 
Index: src/pathchk.c
===================================================================
RCS file: /fetish/cu/src/pathchk.c,v
retrieving revision 1.83
diff -p -u -r1.83 pathchk.c
--- src/pathchk.c       18 Oct 2004 08:19:26 -0000      1.83
+++ src/pathchk.c       10 Jan 2005 18:06:00 -0000
@@ -68,14 +68,21 @@
 # endif
 #endif
 
-static bool validate_file_name (char *file, bool portability);
+static bool validate_file_name (char *, bool, bool);
 
 /* The name this program was run with. */
 char *program_name;
 
+/* For long options that have no equivalent short option, use a
+   non-character as a pseudo short option, starting with CHAR_MAX + 1.  */
+enum
+{
+  PORTABILITY_OPTION = CHAR_MAX + 1
+};
+
 static struct option const longopts[] =
 {
-  {"portability", no_argument, NULL, 'p'},
+  {"portability", no_argument, NULL, PORTABILITY_OPTION},
   {GETOPT_HELP_OPTION_DECL},
   {GETOPT_VERSION_OPTION_DECL},
   {NULL, 0, NULL, 0}
@@ -93,7 +100,9 @@ usage (int status)
       fputs (_("\
 Diagnose unportable constructs in NAME.\n\
 \n\
-  -p, --portability   check for all POSIX systems, not only this one\n\
+  -p                  check for most POSIX systems\n\
+  -P                  check for empty names and leading \"-\"\n\
+      --portability   check for all POSIX systems (equivalent to -p -P)\n\
 "), stdout);
       fputs (HELP_OPTION_DESCRIPTION, stdout);
       fputs (VERSION_OPTION_DESCRIPTION, stdout);
@@ -106,7 +115,8 @@ int
 main (int argc, char **argv)
 {
   bool ok = true;
-  bool check_portability = false;
+  bool check_basic_portability = false;
+  bool check_extra_portability = false;
   int optc;
 
   initialize_main (&argc, &argv);
@@ -117,12 +127,21 @@ main (int argc, char **argv)
 
   atexit (close_stdout);
 
-  while ((optc = getopt_long (argc, argv, "+p", longopts, NULL)) != -1)
+  while ((optc = getopt_long (argc, argv, "+pP", longopts, NULL)) != -1)
     {
       switch (optc)
        {
+       case PORTABILITY_OPTION:
+         check_basic_portability = true;
+         check_extra_portability = true;
+         break;
+
        case 'p':
-         check_portability = true;
+         check_basic_portability = true;
+         break;
+
+       case 'P':
+         check_extra_portability = true;
          break;
 
        case_GETOPT_HELP_CHAR;
@@ -141,11 +160,31 @@ main (int argc, char **argv)
     }
 
   for (; optind < argc; ++optind)
-    ok &= validate_file_name (argv[optind], check_portability);
+    ok &= validate_file_name (argv[optind],
+                             check_basic_portability, check_extra_portability);
 
   exit (ok ? EXIT_SUCCESS : EXIT_FAILURE);
 }
 
+/* If FILE contains a component with a leading "-", report an error
+   and return false; otherwise, return true.  */
+
+static bool
+no_leading_hyphen (char const *file)
+{
+  char const *p;
+
+  for (p = file;  (p = strchr (p, '-'));  p++)
+    if (p == file || p[-1] == '/')
+      {
+       error (0, 0, _("leading `-' in a component of file name %s"),
+              quote (file));
+       return false;
+      }
+
+  return true;
+}
+
 /* If FILE (of length FILELEN) contains only portable characters,
    return true, else report an error and return false.  */
 
@@ -199,18 +238,25 @@ component_len (char const *f)
    strlen (FILE) <= PATH_MAX
    && strlen (each-existing-directory-in-FILE) <= NAME_MAX
 
-   If PORTABILITY is true, compare against _POSIX_PATH_MAX and
+   If CHECK_BASIC_PORTABILITY is true, compare against _POSIX_PATH_MAX and
    _POSIX_NAME_MAX instead, and make sure that FILE contains no
    characters not in the POSIX portable filename character set, which
    consists of A-Z, a-z, 0-9, ., _, - (plus / for separators).
 
-   If PORTABILITY is false, make sure that all leading directories
+   If CHECK_BASIC_PORTABILITY is false, make sure that all leading directories
    along FILE that exist are searchable.
 
+   If CHECK_EXTRA_PORTABILITY is true, check that file name components do not
+   begin with "-".
+
+   If either CHECK_BASIC_PORTABILITY or CHECK_EXTRA_PORTABILITY is true,
+   check that the file name is not empty.
+
    Return true if all of these tests are successful, false if any fail.  */
 
 static bool
-validate_file_name (char *file, bool portability)
+validate_file_name (char *file, bool check_basic_portability,
+                   bool check_extra_portability)
 {
   size_t filelen = strlen (file);
 
@@ -220,17 +266,51 @@ validate_file_name (char *file, bool por
   /* True if component lengths need to be checked.  */
   bool check_component_lengths;
 
-  if (portability && ! portable_chars_only (file, filelen))
+  /* True if the file is known to exist.  */
+  bool file_exists = false;
+
+  if (check_extra_portability && ! no_leading_hyphen (file))
     return false;
 
-  if (*file == '\0')
-    return true;
+  if ((check_basic_portability | check_extra_portability)
+      && filelen == 0)
+    {
+      /* Fail, since empty names are not portable.  As of
+        2005-01-06 POSIX does not address whether "pathchk -p ''"
+        should (or is allowed to) fail, so this is not a
+        conformance violation.  */
+      error (0, 0, _("empty file name"));
+      return false;
+    }
+
+  if (check_basic_portability)
+    {
+      if (! portable_chars_only (file, filelen))
+       return false;
+    }
+  else
+    {
+      /* Check whether a file name component is in a directory that
+        is not searchable, or has some other serious problem.
+        POSIX does not allow "" as a file name, but some non-POSIX
+        hosts do (as an alias for "."), so allow "" if lstat does.  */
+
+      struct stat st;
+      if (lstat (file, &st) == 0)
+       file_exists = true;
+      else if (errno != ENOENT || filelen == 0)
+       {
+         error (0, errno, "%s", file);
+         return false;
+       }
+    }
 
-  if (portability || PATH_MAX_MINIMUM <= filelen)
+  if (check_basic_portability
+      || (! file_exists && PATH_MAX_MINIMUM <= filelen))
     {
       size_t maxsize;
 
-      if (portability)
+      if (check_basic_portability)
        maxsize = _POSIX_PATH_MAX;
       else
        {
@@ -258,26 +338,13 @@ validate_file_name (char *file, bool por
        }
     }
 
-  if (! portability)
-    {
-      /* Check whether a file name component is in a directory that
-        is not searchable, or has some other serious problem.  */
-
-      struct stat st;
-      if (lstat (file, &st) != 0 && errno != ENOENT)
-       {
-         error (0, errno, "%s", file);
-         return false;
-       }
-    }
-
   /* Check whether pathconf (..., _PC_NAME_MAX) can be avoided, i.e.,
      whether all file name components are so short that they are valid
-     in any file system on this platform.  If PORTABILITY, though,
+     in any file system on this platform.  If CHECK_BASIC_PORTABILITY, though,
      it's more convenient to check component lengths below.  */
 
-  check_component_lengths = portability;
-  if (! check_component_lengths)
+  check_component_lengths = check_basic_portability;
+  if (! check_component_lengths && ! file_exists)
     {
       for (start = file; *(start = component_start (start)); )
        {
@@ -302,7 +369,7 @@ validate_file_name (char *file, bool por
       size_t name_max = NAME_MAX_MINIMUM;
 
       /* If nonzero, the known limit on file name components.  */
-      size_t known_name_max = (portability ? _POSIX_NAME_MAX : 0);
+      size_t known_name_max = (check_basic_portability ? _POSIX_NAME_MAX : 0);
 
       for (start = file; *(start = component_start (start)); )
        {
Index: tests/misc/pathchk1
===================================================================
RCS file: /fetish/cu/tests/misc/pathchk1,v
retrieving revision 1.3
diff -p -u -r1.3 pathchk1
--- tests/misc/pathchk1 23 Jun 2004 15:07:04 -0000      1.3
+++ tests/misc/pathchk1 10 Jan 2005 18:06:00 -0000
@@ -1,5 +1,5 @@
 #!/bin/sh
-# ensure that pathchk exits nonzero in one particular case where it didn't
+# pathchk tests
 
 if test "$VERBOSE" = yes; then
   set -x
@@ -29,4 +29,12 @@ fail=0
 # but exited successfully.
 pathchk file/x > /dev/null 2>&1 && fail=1
 
+# This should exit nonzero.  Through 5.3.0 it exited with status zero.
+pathchk -p '' > /dev/null 2>&1 && fail=1
+
+# This tests the new -P option.
+pathchk -P '' > /dev/null 2>&1 && fail=1
+pathchk -P -- - > /dev/null 2>&1 && fail=1
+pathchk -p -P x/- > /dev/null 2>&1 && fail=1
+
 (exit $fail); exit $fail