avoid fopen bugs

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Two patches.  First, avoid fopen bugs by using gnulib (Irix, older
Solaris, and HP-UX had the annoying tendency to improperly ignore trailing
slashes).

Next, POSIX states that m4 is only required to operate on text files;
directories are not text files, so we can handle them however we'd like.
Most sane systems allow fopen(".","r") (required by POSIX), but then fail
with EISDIR when trying to read that stream; BSD treats the stream like an
empty file; but then there are systems like EMX OS/2 that read the
directory contents as though it were a text file, or mingw that forbids
opening a directory.  Thus, it is a lot nicer if we blindly reject all
attempts to read a directory, for cross-platform consistency.

In both cases, mingw does not use the typical errno value.  So I had to
modify the testsuite to allow ignoring stderr, so as to avoid failures
from the difference in strerror output.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkjbEIwACgkQ84KuGfSFAYBx1ACgzJpmo+O6qzLmnJG3eTOlaqHd
wc0An2OAcQeltVI4m1gnjPAP1eK9Sv3F
=h/fr
-----END PGP SIGNATURE-----

>From ce1f8f6861b928b7b587d69eee2000a0101e3381 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Wed, 24 Sep 2008 20:41:05 -0600
Subject: [PATCH] Avoid bugs on platforms that mishandle trailing /.

* m4/gnulib-cache.m4: Import fopen module.
* doc/m4.texinfo (Command line files, Include): Add tests.
(Using frozen files): This test now works on mingw.
* checks/check-them (examples): Expand xerr to allow ignoring
error output because of differences in platform errno values.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog          |    9 +++++++++
 checks/check-them  |   19 +++++++++++--------
 doc/m4.texinfo     |   36 +++++++++++++++++++++++++++++++++---
 m4/gnulib-cache.m4 |    3 ++-
 4 files changed, 55 insertions(+), 12 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 69969b0..5660340 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2008-09-24  Eric Blake  <address@hidden>
+
+       Avoid bugs on platforms that mishandle trailing /.
+       * m4/gnulib-cache.m4: Import fopen module.
+       * doc/m4.texinfo (Command line files, Include): Add tests.
+       (Using frozen files): This test now works on mingw.
+       * checks/check-them (examples): Expand xerr to allow ignoring
+       error output because of differences in platform errno values.
+
 2008-09-22  Eric Blake  <address@hidden>
 
        Support alternate path separator.
diff --git a/checks/check-them b/checks/check-them
index fac1b2b..46162ca 100755
--- a/checks/check-them
+++ b/checks/check-them
@@ -126,14 +126,17 @@ do
   fi
 
   xerrfile=`sed -n 's/^dnl @ expected error: //p' "$file"`
-  if test -z "$xerrfile" ; then
-    sed '/^dnl @error{}/!d
-        s///; '"s|^m4:|$m4name:|; s|examples/|$examples/|" \
-      "$file" > $xerr
-  else
-    sed "s|^m4:|$m4name:|; s|examples/|$examples/|" \
-      "$examples/$xerrfile" > $xerr
-  fi
+  case $xerrfile in
+    ignore)
+      cp $err $xerr ;;
+    '')
+      sed '/^dnl @error{}/!d
+          s///; '"s|^m4:|$m4name:|; s|examples/|$examples/|" \
+        "$file" > $xerr ;;
+    *)
+      sed "s|^m4:|$m4name:|; s|examples/|$examples/|" \
+        "$examples/$xerrfile" > $xerr ;;
+  esac
 
   # For the benefit of mingw, normalize \r\n line endings
   if $strip_needed ; then
diff --git a/doc/m4.texinfo b/doc/m4.texinfo
index ff653c2..7ad39dd 100644
--- a/doc/m4.texinfo
+++ b/doc/m4.texinfo
@@ -925,6 +925,18 @@ Command line files
 specify it as @samp{./-file}, or use @option{--} to mark the end of
 options.
 
address@hidden
address@hidden Test that 'm4 file/' detects that file is not a directory; we
address@hidden can assume that the current directory contains a Makefile.
+
address@hidden status: 1
address@hidden xerr: ignore
address@hidden options: Makefile/
address@hidden
address@hidden: Makefile/: Not a directory
address@hidden example
address@hidden ignore
+
 @node Syntax
 @chapter Lexical and syntactic conventions
 
@@ -4774,6 +4786,27 @@ Include
 In @acronym{GNU} @code{m4}, an alternative method of reading files is
 using @code{undivert} (@pxref{Undivert}) on a named file.
 
address@hidden
address@hidden Test that include(`file/') detects that file is not a
address@hidden directory; we can assume that the current directory contains a
address@hidden Makefile.
+
address@hidden status: 1
address@hidden xerr: ignore
address@hidden
+include(`Makefile/')
address@hidden:stdin:1: cannot open `Makefile/': Not a directory
address@hidden
address@hidden example
+
address@hidden Meanwhile, ignore errors with sinclude.
+
address@hidden
+sinclude(`Makefile/')
address@hidden
address@hidden example
address@hidden ignore
+
 @node Search Path
 @section Searching for include files
 
@@ -6767,9 +6800,6 @@ Using frozen files
 
 @comment options: -F /dev/null
 @example
-ifdef(`__unix__', ,
-      `errprint(` skipping: /dev/null not known to exist
-')m4exit(`77')')dnl
 traceon(`undefined')dnl
 @end example
 
diff --git a/m4/gnulib-cache.m4 b/m4/gnulib-cache.m4
index 49f7c18..96864de 100644
--- a/m4/gnulib-cache.m4
+++ b/m4/gnulib-cache.m4
@@ -15,7 +15,7 @@
 
 
 # Specification in the form of a command-line invocation:
-#   gnulib-tool --import --dir=. --local-dir=local --lib=libm4 
--source-base=lib --m4-base=m4 --doc-base=doc --aux-dir=build-aux --with-tests 
--no-libtool --macro-prefix=M4 announce-gen assert autobuild avltree-oset 
binary-io c-stack clean-temp cloexec close-stream closein config-h dirname 
error fdl fflush filenamecat fopen-safer fseeko gendocs getopt git-version-gen 
gnumakefile gnupload gpl-3.0 intprops mkstemp obstack progname regex sigaction 
stdbool stdint stdlib-safer strsignal strstr strtod strtol unlocked-io verror 
version-etc version-etc-fsf xalloc xprintf xvasprintf-posix
+#   gnulib-tool --import --dir=. --local-dir=local --lib=libm4 
--source-base=lib --m4-base=m4 --doc-base=doc --aux-dir=build-aux --with-tests 
--no-libtool --macro-prefix=M4 announce-gen assert autobuild avltree-oset 
binary-io c-stack clean-temp cloexec close-stream closein config-h dirname 
error fdl fflush filenamecat fopen fopen-safer fseeko gendocs getopt 
git-version-gen gnumakefile gnupload gpl-3.0 intprops mkstemp obstack progname 
regex sigaction stdbool stdint stdlib-safer strsignal strstr strtod strtol 
unlocked-io verror version-etc version-etc-fsf xalloc xprintf xvasprintf-posix
 
 # Specification in the form of a few gnulib-tool.m4 macro invocations:
 gl_LOCAL_DIR([local])
@@ -36,6 +36,7 @@ gl_MODULES([
   fdl
   fflush
   filenamecat
+  fopen
   fopen-safer
   fseeko
   gendocs
-- 
1.6.0.2


>From eed62f0d2729c243d7c081c48d789f9d86fa340f Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Wed, 24 Sep 2008 21:42:53 -0600
Subject: [PATCH] Unify error handling for reading directories.

* src/path.c (m4_path_search): Factor open attempts...
(m4_fopen): ...into new function, to reject directories.
* doc/m4.texinfo (Include): Document that directories cannot be
input files.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog      |    6 ++++++
 doc/m4.texinfo |   21 +++++++++++++++++++--
 src/path.c     |   34 +++++++++++++++++++++++++---------
 3 files changed, 50 insertions(+), 11 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 5660340..b2e6b57 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2008-09-24  Eric Blake  <address@hidden>
 
+       Unify error handling for reading directories.
+       * src/path.c (m4_path_search): Factor open attempts...
+       (m4_fopen): ...into new function, to reject directories.
+       * doc/m4.texinfo (Include): Document that directories cannot be
+       input files.
+
        Avoid bugs on platforms that mishandle trailing /.
        * m4/gnulib-cache.m4: Import fopen module.
        * doc/m4.texinfo (Command line files, Include): Add tests.
diff --git a/doc/m4.texinfo b/doc/m4.texinfo
index 7ad39dd..6fcd9f3 100644
--- a/doc/m4.texinfo
+++ b/doc/m4.texinfo
@@ -928,6 +928,7 @@ Command line files
 @ignore
 @comment Test that 'm4 file/' detects that file is not a directory; we
 @comment can assume that the current directory contains a Makefile.
address@hidden mingw fails with EINVAL rather than ENOTDIR.
 
 @comment status: 1
 @comment xerr: ignore
@@ -4705,7 +4706,8 @@ Include
 The expansion of @code{include} and @code{sinclude} is therefore the
 contents of @var{file}.
 
-If @var{file} does not exist (or cannot be read), the expansion is void,
+If @var{file} does not exist, is a directory, or cannot otherwise be
+read, the expansion is void,
 and @code{include} will fail with an error while @code{sinclude} is
 silent.  The empty string counts as a file that does not exist.
 
@@ -4789,7 +4791,7 @@ Include
 @ignore
 @comment Test that include(`file/') detects that file is not a
 @comment directory; we can assume that the current directory contains a
address@hidden Makefile.
address@hidden Makefile.  mingw fails with EINVAL rather than ENOTDIR.
 
 @comment status: 1
 @comment xerr: ignore
@@ -4799,11 +4801,26 @@ Include
 @result{}
 @end example
 
address@hidden POSIX allows, but doesn't require, failure on reading
address@hidden directories.  But since they aren't text files, it never makes
address@hidden sense, so we globally forbid it even if fopen doesn't.  mingw
address@hidden fails with EACCES rather than EISDIR.
+
address@hidden status: 1
address@hidden xerr: ignore
address@hidden
+include(`.')
address@hidden:stdin:1: cannot open `.': Is a directory
address@hidden
address@hidden example
+
 @comment Meanwhile, ignore errors with sinclude.
 
 @example
 sinclude(`Makefile/')
 @result{}
+sinclude(`.')
address@hidden
 @end example
 @end ignore
 
diff --git a/src/path.c b/src/path.c
index 4b904fc..317bd4e 100644
--- a/src/path.c
+++ b/src/path.c
@@ -105,6 +105,29 @@ add_include_directory (const char *dir)
 #endif
 }
 
+/* Attempt to open FILE; if it opens, verify that it is not a
+   directory, and ensure it does not leak across execs.  */
+static FILE *
+m4_fopen (const char *file, const char *mode)
+{
+  FILE *fp = fopen (file, "r");
+  if (fp)
+    {
+      struct stat st;
+      int fd = fileno (fp);
+      if (fstat (fd, &st) == 0 && S_ISDIR (st.st_mode))
+       {
+         fclose (fp);
+         errno = EISDIR;
+         return NULL;
+       }
+      if (set_cloexec_flag (fd, true) != 0)
+       M4ERROR ((warning_status, errno,
+                 "Warning: cannot protect input file across forks"));
+    }
+  return fp;
+}
+
 /* Search for FILE, first in `.', then according to -I options.  If
    successful, return the open file, and if RESULT is not NULL, set
    *RESULT to a malloc'd string that represents the file found with
@@ -129,12 +152,9 @@ m4_path_search (const char *file, char **result)
     }
 
   /* Look in current working directory first.  */
-  fp = fopen (file, "r");
+  fp = m4_fopen (file, "r");
   if (fp != NULL)
     {
-      if (set_cloexec_flag (fileno (fp), true) != 0)
-       M4ERROR ((warning_status, errno,
-                 "Warning: cannot protect input file across forks"));
       if (result)
        *result = xstrdup (file);
       return fp;
@@ -153,19 +173,15 @@ m4_path_search (const char *file, char **result)
       xfprintf (stderr, "m4_path_search (%s) -- trying %s\n", file, name);
 #endif
 
-      fp = fopen (name, "r");
+      fp = m4_fopen (name, "r");
       if (fp != NULL)
        {
          if (debug_level & DEBUG_TRACE_PATH)
            DEBUG_MESSAGE2 ("path search for `%s' found `%s'", file, name);
-         if (set_cloexec_flag (fileno (fp), true) != 0)
-           M4ERROR ((warning_status, errno,
-                     "Warning: cannot protect input file across forks"));
          if (result)
            *result = name;
          else
            free (name);
-         errno = e;
          return fp;
        }
       free (name);
-- 
1.6.0.2