avoid fopen bugs

From ce1f8f6861b928b7b587d69eee2000a0101e3381 Mon Sep 17 00:00:00 2001
From: Eric Blake <[email protected]>
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 <[email protected]>
---
 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  <[email protected]>
+
+	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  <[email protected]>
 
 	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.
 
+@ignore
+@comment Test that 'm4 file/' detects that file is not a directory; we
+@comment can assume that the current directory contains a Makefile.
+
+@comment status: 1
+@comment xerr: ignore
+@comment options: Makefile/
+@example
+@error{}m4: Makefile/: Not a directory
+@end example
+@end 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.
 
+@ignore
+@comment Test that include(`file/') detects that file is not a
+@comment directory; we can assume that the current directory contains a
+@comment Makefile.
+
+@comment status: 1
+@comment xerr: ignore
+@example
+include(`Makefile/')
+@error{}m4:stdin:1: cannot open `Makefile/': Not a directory
+@result{}
+@end example
+
+@comment Meanwhile, ignore errors with sinclude.
+
+@example
+sinclude(`Makefile/')
+@result{}
+@end example
+@end 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 <[email protected]>
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 <[email protected]>
---
 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  <[email protected]>
 
+	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.
+@comment 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
-@comment Makefile.
+@comment Makefile.  mingw fails with EINVAL rather than ENOTDIR.
 
 @comment status: 1
 @comment xerr: ignore
@@ -4799,11 +4801,26 @@ Include
 @result{}
 @end example
 
+@comment POSIX allows, but doesn't require, failure on reading
+@comment directories.  But since they aren't text files, it never makes
+@comment sense, so we globally forbid it even if fopen doesn't.  mingw
+@comment fails with EACCES rather than EISDIR.
+
+@comment status: 1
+@comment xerr: ignore
+@example
+include(`.')
+@error{}m4:stdin:1: cannot open `.': Is a directory
+@result{}
+@end example
+
 @comment Meanwhile, ignore errors with sinclude.
 
 @example
 sinclude(`Makefile/')
 @result{}
+sinclude(`.')
+@result{}
 @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