head: m4 -otrace --help

This bug affects both branch and head, but I don't want to waste effort on 
creating a 1.4.8 unless I am fixing multiple bugs, and this one is relatively 
minor.

$ rm -f trace
$ m4 --debugfile=trace --help >/dev/null
$ ls -l trace
-rw-r--r-- 1 eblake eblake 0 Sep 29 11:25 trace

Oops - we created the debug file before realizing that we weren't going to 
generate trace data because of --help.  My scan of main() did not turn up any 
other permanent side effect interactions with --help or --version.  On the 
other hand, I can live with behavior like this:

$ m4 -S1 --version
m4: Warning: `m4 -S' may be removed in a future release
GNU M4 1.4.7
Copyright (C) 2006 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Written by Rene' Seindal.

The debug file change still has to take place quite early, so that module 
loading traces go to the right place.  I also fixed it up so that we don't 
close the trace file too early when tracing modules (a bug on head only).


2006-09-29  Eric Blake  <[email protected]>

	Even when not the first option, --help can't have side effects.
	* tests/options.at (--debugfile): Detect bugs in this area.
	* src/main.c (main): Fix it by deferring debugfile change.  Also,
	defer closing streams until after module tracing is done.
	* modules/gnu.c (debugfile): Make message consistent with command
	line.
	* doc/m4.texinfo (Invoking m4): Touch up the documentation.
	* NEWS: Document this fix.

Index: NEWS
===================================================================
RCS file: /sources/m4/m4/NEWS,v
retrieving revision 1.23
diff -u -r1.23 NEWS
--- NEWS	29 Sep 2006 12:26:06 -0000	1.23
+++ NEWS	29 Sep 2006 18:04:42 -0000
@@ -105,6 +105,10 @@
   overflow.  Also, the option now performs argument validation and accepts
   an optional multiplier suffix.
 
+* The `--help' and `--version' command line options now consistently
+  override all earlier options.  For example, `m4 -otrace --help' now no
+  longer accidentally creates an empty file `trace'.
+
 * FIXME: `m4wrap' semantics need an update to FIFO.
 
 * FIXME: include the (long) list of changes in 1.4.x that were not already
Index: doc/m4.texinfo
===================================================================
RCS file: /sources/m4/m4/doc/m4.texinfo,v
retrieving revision 1.56
diff -u -r1.56 m4.texinfo
--- doc/m4.texinfo	29 Sep 2006 13:33:13 -0000	1.56
+++ doc/m4.texinfo	29 Sep 2006 18:04:42 -0000
@@ -428,11 +428,13 @@
 @table @code
 @item --help
 Print a help summary on standard output, then immediately exit
-@code{m4} without reading any input files.
+@code{m4} without reading any input files or performing any other
+actions.
 
 @item --version
 Print the version number of the program on standard output, then
-immediately exit @code{m4} without reading any input files.
+immediately exit @code{m4} without reading any input files or
+performing any other actions.
 
 @item -b
 @itemx --batch
Index: modules/gnu.c
===================================================================
RCS file: /sources/m4/m4/modules/gnu.c,v
retrieving revision 1.58
diff -u -r1.58 gnu.c
--- modules/gnu.c	27 Sep 2006 12:24:53 -0000	1.58
+++ modules/gnu.c	29 Sep 2006 18:04:42 -0000
@@ -404,7 +404,7 @@
   else if (m4_get_safer_opt (context) && *M4ARG (1))
     m4_error (context, 0, 0, _("%s: disabled by --safer"), M4ARG (0));
   else if (!m4_debug_set_output (context, M4ARG (1)))
-    m4_error (context, 0, errno, _("%s: cannot set debug file: %s"),
+    m4_error (context, 0, errno, _("%s: cannot set debug file `%s'"),
 	      M4ARG (0), M4ARG (1));
 }
 
Index: src/main.c
===================================================================
RCS file: /sources/m4/m4/src/main.c,v
retrieving revision 1.90
diff -u -r1.90 main.c
--- src/main.c	29 Sep 2006 13:33:14 -0000	1.90
+++ src/main.c	29 Sep 2006 18:04:42 -0000
@@ -256,17 +256,17 @@
 int
 main (int argc, char *const *argv, char *const *envp)
 {
-  macro_definition *head;	/* head of deferred argument list */
-  macro_definition *tail;
+  macro_definition *head = NULL;	/* head of deferred argument list */
+  macro_definition *tail = NULL;
   macro_definition *defn;
   int optchar;			/* option character */
   size_t size;			/* for parsing numeric option arguments */
 
   macro_definition *defines;
   FILE *fp;
-  char *filename;
   bool read_stdin = false;	/* true iff we have read from stdin */
   bool import_environment = false; /* true to import environment */
+  const char *debugfile = NULL;
   const char *frozen_file_to_read = NULL;
   const char *frozen_file_to_write = NULL;
   enum interactive_choice interactive = INTERACTIVE_UNKNOWN;
@@ -296,10 +296,9 @@
   if (getenv ("POSIXLY_CORRECT"))
     m4_set_posixly_correct_opt (context, true);
 
-  /* First, we decode the arguments, to size up tables and stuff.  */
-
-  head = tail = NULL;
-
+  /* First, we decode the arguments, to size up tables and stuff.
+     Avoid lasting side effects; for example 'm4 --debugfile=oops
+     --help' must not create the file `oops'.  */
   while ((optchar = getopt_long (argc, (char **) argv, OPTSTRING,
 				 long_options, NULL)) != -1)
     switch (optchar)
@@ -465,8 +464,8 @@
 	       optchar == 'o' ? "-o" : "--error-output", "--debugfile");
 	/* fall through */
       case DEBUGFILE_OPTION:
-	if (!m4_debug_set_output (context, optarg))
-	  error (0, errno, "%s", optarg);
+	/* Don't call m4_debug_set_output here, as it has side effects.	 */
+	debugfile = optarg;
 	break;
 
       case 's':
@@ -502,7 +501,8 @@
 					&& isatty (STDERR_FILENO))));
 
   /* Do the basic initializations.  */
-
+  if (debugfile && !m4_debug_set_output (context, debugfile))
+    m4_error (context, 0, errno, _("cannot set debug file `%s'"), debugfile);
   m4_input_init (context);
   m4_output_init ();
   m4_include_env_init (context);
@@ -625,7 +625,8 @@
 	  }
 	else
 	  {
-	    fp = m4_path_search (context, argv[optind], &filename);
+	    char *name;
+	    fp = m4_path_search (context, argv[optind], &name);
 	    if (fp == NULL)
 	      {
 		error (0, errno, "%s", argv[optind]);
@@ -634,8 +635,8 @@
 	      }
 	    else
 	      {
-		m4_push_file (context, fp, filename, true);
-		free (filename);
+		m4_push_file (context, fp, name, true);
+		free (name);
 	      }
 	  }
 	m4_macro_expand_input (context);
@@ -645,13 +646,6 @@
   while (m4_pop_wrapup ())
     m4_macro_expand_input (context);
 
-  /* Change debug stream back to stderr, to force flushing the debug
-     stream and detect any errors it might have encountered.  Close
-     stdin if we read from it, to detect any errors.  */
-  m4_debug_set_output (context, NULL);
-  if (read_stdin && fclose (stdin) == EOF)
-    m4_error (context, 0, errno, _("error closing stdin"));
-
   if (frozen_file_to_write)
     produce_frozen_state (context, frozen_file_to_write);
   else
@@ -670,6 +664,13 @@
   m4_output_exit ();
   m4_input_exit ();
 
+  /* Change debug stream back to stderr, to force flushing the debug
+     stream and detect any errors it might have encountered.  Close
+     stdin if we read from it, to detect any errors.  */
+  m4_debug_set_output (context, NULL);
+  if (read_stdin && fclose (stdin) == EOF)
+    m4_error (context, 0, errno, _("error closing stdin"));
+
   if (exit_status == EXIT_SUCCESS)
     exit_status = m4_get_exit_status (context);
   m4_delete (context);
Index: tests/options.at
===================================================================
RCS file: /sources/m4/m4/tests/options.at,v
retrieving revision 1.14
diff -u -r1.14 options.at
--- tests/options.at	29 Sep 2006 12:26:07 -0000	1.14
+++ tests/options.at	29 Sep 2006 18:04:42 -0000
@@ -239,7 +239,7 @@
 
 AT_SETUP([--debugfile])
 
-dnl For a while, CVS m4 mistakenly replaced debug output with stdout
+dnl For a while, CVS m4 mistakenly sent debug output to stdout
 dnl when stdout and stderr were the same file.
 
 AT_DATA([[in]], [[foo
@@ -253,6 +253,34 @@
 
 AT_CHECK([cmp trace1 trace2])
 
+dnl m4 1.4.x mistakenly created the trace file.  --help and --version
+dnl should always override, even if they come later in the command line
+AT_CHECK_M4([--debugfile=trace3 --help], [0], [ignore])
+AT_CHECK_M4([--debugfile=trace3 --version], [0], [ignore])
+AT_CHECK([test -f trace3], [1])
+
+dnl check that trace file failure causes an error, but allows processing
+AT_CHECK_M4([--debugfile=no_such_dir/trace -tfoo -Dfoo=bar in], [1],
+[[bar
+]], [[m4: cannot set debug file `no_such_dir/trace': No such file or directory
+m4trace: -1- foo -> `bar'
+]])
+
+dnl check that empty trace file discards trace data
+AT_CHECK_M4([--debugfile= -tfoo -Dfoo=bar in], [0],
+[[bar
+]])
+
+dnl check that all tracing gets diverted to the trace file.  Don't use
+dnl AT_CHECK_M4 on the first run, because sanitizing stderr breaks the
+dnl comparison with the raw data in the trace file of the second run.
+AT_CHECK([m4 -b -dV in], [0], [[foo
+]], [stderr])
+mv stderr expout
+AT_CHECK_M4([--debugfile=trace4 -dV in], [0], [[foo
+]])
+AT_CHECK([cat trace4], [0], [expout])
+
 AT_CLEANUP