--debugfile behavior

[Eric Blake]

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

While working on adding debugmode(`+o') for branch-1.6, I noticed that
there is no command-line way to change the current debug file to stderr if
it has been changed elsewhere (at least, not without OS support for
/dev/stderr or /proc/self/fd/2...).  I introduced --debugfile in m4 1.4.7
with a mandatory argument 2 years ago, but think that an optional argument
would serve us better; the only drawback is that anyone using the
non-canonical form 'm4 --debugfile trace' instead of following the
recommended 'm4 --debugfile=trace' will be broken.  But since --debugfile
isn't that old, and the primary user that I am aware of (autoconf) uses
the correct form, I'm okay with making this change.  The older spellings
of --error-output and -o options do not change in behavior for branch-1.4,
although I'm thinking that 2 years has been long enough with --debugfile
available that I can make branch-1.6 issue a deprecation warning for those
(and thus open the door for m4 2.0 to use -o to specify where output goes,
rather than where traces are dumped, consistent with other GNU programs
like gcc).

- --
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

iEYEARECAAYFAklPnWIACgkQ84KuGfSFAYBO0ACgpto1Yzkc1tHnEht/9TcN0Hl5
xkkAn3VU7hnXptxFARNMhssN6VdpLOw6
=dHEF
-----END PGP SIGNATURE-----

>From b2d01854ce7a525c38c5cb76c24401766343ab2f Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Mon, 22 Dec 2008 06:50:38 -0700
Subject: [PATCH] Make --debugfile argument optional.

* src/builtin.c (m4_debugfile): Make error message consistent.
* src/m4.c (long_options): Make the argument optional, to allow
setting debug file back to stderr.
(main): Make --debugfile order-dependent.
(usage): Document this.
* doc/m4.texinfo (Debugging options): Likewise.
* NEWS: Likewise.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog      |   11 +++++++++
 NEWS           |    7 ++++++
 doc/m4.texinfo |   32 +++++++++++++++++++++++-----
 src/builtin.c  |    2 +-
 src/m4.c       |   63 +++++++++++++++++++++++++++++++------------------------
 5 files changed, 80 insertions(+), 35 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 802ee0a..b094064 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2008-12-22  Eric Blake  <address@hidden>
+
+       Make --debugfile argument optional.
+       * src/builtin.c (m4_debugfile): Make error message consistent.
+       * src/m4.c (long_options): Make the argument optional, to allow
+       setting debug file back to stderr.
+       (main): Make --debugfile order-dependent.
+       (usage): Document this.
+       * doc/m4.texinfo (Debugging options): Likewise.
+       * NEWS: Likewise.
+
 2008-12-18  Eric Blake  <address@hidden>
 
        Document optimized forloop.
diff --git a/NEWS b/NEWS
index 1be2c28..906601f 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,13 @@ Foundation, Inc.
 ** The `divert' and `undivert' builtins have been made more efficient
    when using temporary files for large diversions.
 
+** The command line option `--debugfile', introduced in 1.4.7, now
+   treats its argument as optional, in order to allow setting the debug
+   output back to stderr when used without an argument; and order is now
+   significant with respect to command line files.  You must therefore use
+   `m4 --debugfile=trace file', not `m4 file --debugfile trace'.  This
+   change does not affect the deprecated `-o'/`--error-output' option.
+
 ** A number of portability improvements inherited from gnulib.
 
 * Noteworthy changes in Version 1.4.12 (2008-10-10) [stable]
diff --git a/doc/m4.texinfo b/doc/m4.texinfo
index c5bdc30..2d84ca6 100644
--- a/doc/m4.texinfo
+++ b/doc/m4.texinfo
@@ -853,18 +853,38 @@ Debugging options
 functions.  @xref{Debug Levels}, for more details on the format and
 meaning of @var{FLAGS}.  If omitted, @var{FLAGS} defaults to @samp{aeq}.
 
address@hidden address@hidden
address@hidden address@hidden@address@hidden
 @itemx -o @var{FILE}
 @itemx address@hidden
 Redirect @code{dumpdef} output, debug messages, and trace output to the
 named @var{FILE}.  Warnings, error messages, and @code{errprint} output
-are still printed to standard error.  If unspecified, debug output goes
-to standard error; if empty, debug output is discarded.  @xref{Debug
-Output}, for more details.  The spellings @option{-o} and
+are still printed to standard error.  If these options are not used, or
+if @var{FILE} is unspecified (only possible for @option{--debugfile}),
+debug output goes to standard error; if @var{FILE} is empty, debug
+output is discarded.  @xref{Debug Output}, for more details.  The option
address@hidden may be given more than once, and order is
+significant with respect to file names.  The spellings @option{-o} and
 @option{--error-output} are misleading and inconsistent with other
 @acronym{GNU} tools; for now they are silently accepted as synonyms of
address@hidden, but in a future version of M4, using them will
-cause a warning to be issued.
address@hidden and only recognized once, but in a future version
+of M4, using them will cause a warning to be issued.
+
address@hidden
address@hidden not worth including in the manual, but provides a good test
+
address@hidden examples
address@hidden options: -Dbar=hello -tbar --debugfile= foo --debugfile -
address@hidden
+$ @kbd{m4 -d -tbar --debugfile= foo --debugfile -
address@hidden
+errprint(`hi
+')dnl
address@hidden
+bar
address@hidden: -1- bar -> `hello'
address@hidden
address@hidden example
address@hidden ignore
 
 @item -l @var{NUM}
 @itemx address@hidden
diff --git a/src/builtin.c b/src/builtin.c
index 03bb649..50f10cd 100644
--- a/src/builtin.c
+++ b/src/builtin.c
@@ -1662,7 +1662,7 @@ m4_debugfile (struct obstack *obs, int argc, token_data 
**argv)
     debug_set_output (NULL);
   else if (!debug_set_output (ARG (1)))
     M4ERROR ((warning_status, errno,
-             "cannot set error file: `%s'", ARG (1)));
+             "cannot set debug file `%s'", ARG (1)));
 }
 
 /* This section contains text processing macros: "len", "index",
diff --git a/src/m4.c b/src/m4.c
index befc7a9..b06044d 100644
--- a/src/m4.c
+++ b/src/m4.c
@@ -125,7 +125,7 @@ m4_error_at_line (int status, int errnum, const char *file, 
int line,
 #  define MAX(a,b) ((a) < (b) ? (b) : (a))
 # endif
 # define NSIG (MAX (SIGABRT, MAX (SIGILL, MAX (SIGFPE,  \
-                                               MAX (SIGSEGV, SIGBUS)))) + 1)
+                                              MAX (SIGSEGV, SIGBUS)))) + 1)
 #endif
 
 /* Pre-translated messages for program errors.  Do not translate in
@@ -144,20 +144,20 @@ fault_handler (int signo)
   if (signo)
     {
       /* POSIX states that reading static memory is, in general, not
-         async-safe.  However, the static variables that we read are
-         never modified once this handler is installed, so this
-         particular usage is safe.  And it seems an oversight that
-         POSIX claims strlen is not async-safe.  */
+        async-safe.  However, the static variables that we read are
+        never modified once this handler is installed, so this
+        particular usage is safe.  And it seems an oversight that
+        POSIX claims strlen is not async-safe.  */
       write (STDERR_FILENO, program_name, strlen (program_name));
       write (STDERR_FILENO, ": ", 2);
       write (STDERR_FILENO, program_error_message,
-             strlen (program_error_message));
+            strlen (program_error_message));
       if (signal_message[signo])
-        {
-          write (STDERR_FILENO, ": ", 2);
-          write (STDERR_FILENO, signal_message[signo],
-                 strlen (signal_message[signo]));
-        }
+       {
+         write (STDERR_FILENO, ": ", 2);
+         write (STDERR_FILENO, signal_message[signo],
+                strlen (signal_message[signo]));
+       }
       write (STDERR_FILENO, "\n", 1);
       _exit (EXIT_INTERNAL_ERROR);
     }
@@ -191,13 +191,13 @@ Operation modes:\n\
 ", stdout);
       xprintf ("\
   -E, --fatal-warnings         once: warnings become errors, twice: stop\n\
-                               execution at first error\n\
+                                 execution at first error\n\
   -i, --interactive            unbuffer output, ignore interrupts\n\
   -P, --prefix-builtins        force a `m4_' prefix to all builtins\n\
   -Q, --quiet, --silent        suppress some warnings for builtins\n\
       --warn-macro-sequence[=REGEXP]\n\
                                warn if macro definition matches REGEXP,\n\
-                               default %s\n\
+                                 default %s\n\
 ", DEFAULT_MACRO_SEQUENCE);
 #ifdef ENABLE_CHANGEWORD
       fputs ("\
@@ -230,7 +230,8 @@ Frozen state files:\n\
 \n\
 Debugging:\n\
   -d, --debug[=FLAGS]          set debug level (no FLAGS implies `aeq')\n\
-      --debugfile=FILE         redirect debug and trace output\n\
+      --debugfile[=FILE]       redirect debug and trace output to FILE\n\
+                                 (default stderr, discard if empty string)\n\
   -l, --arglength=NUM          restrict macro tracing size\n\
   -t, --trace=NAME             trace NAME when it is defined\n\
 ", stdout);
@@ -303,7 +304,7 @@ static const struct option long_options[] =
   {"undefine", required_argument, NULL, 'U'},
   {"word-regexp", required_argument, NULL, 'W'},
 
-  {"debugfile", required_argument, NULL, DEBUGFILE_OPTION},
+  {"debugfile", optional_argument, NULL, DEBUGFILE_OPTION},
   {"diversions", required_argument, NULL, DIVERSIONS_OPTION},
   {"warn-macro-sequence", optional_argument, NULL, WARN_MACRO_SEQUENCE_OPTION},
 
@@ -387,7 +388,7 @@ main (int argc, char *const *argv, char *const *envp)
      handlers.  */
   program_error_message
     = xasprintf (_("internal error detected; please report this bug to <%s>"),
-                 PACKAGE_BUGREPORT);
+                PACKAGE_BUGREPORT);
   signal_message[SIGSEGV] = xstrdup (strsignal (SIGSEGV));
   signal_message[SIGABRT] = xstrdup (strsignal (SIGABRT));
   signal_message[SIGILL] = xstrdup (strsignal (SIGILL));
@@ -415,10 +416,10 @@ main (int argc, char *const *argv, char *const *envp)
     char *crash = getenv ("M4_CRASH");
     if (crash)
       {
-        if (!atoi (crash))
-          ++*(int *) 8;
-        assert (false);
-        abort ();
+       if (!atoi (crash))
+         ++*(int *) 8;
+       assert (false);
+       abort ();
       }
   }
 #endif /* DEBUG_STKOVF */
@@ -455,6 +456,7 @@ main (int argc, char *const *argv, char *const *envp)
       case 's':
       case 't':
       case '\1':
+      case DEBUGFILE_OPTION:
        /* Arguments that cannot be handled until later are accumulated.  */
 
        defn = (macro_definition *) xmalloc (sizeof (macro_definition));
@@ -548,17 +550,16 @@ main (int argc, char *const *argv, char *const *envp)
           but don't issue a deprecation warning until autoconf 2.61
           or later is more widely established, as such a warning
           would interfere with all earlier versions of autoconf.  */
-      case DEBUGFILE_OPTION:
        /* Don't call debug_set_output here, as it has side effects.  */
        debugfile = optarg;
        break;
 
       case WARN_MACRO_SEQUENCE_OPTION:
-         /* Don't call set_macro_sequence here, as it can exit.
-            --warn-macro-sequence sets optarg to NULL (which uses the
-            default regexp); --warn-macro-sequence= sets optarg to ""
-            (which disables these warnings).  */
-        macro_sequence = optarg;
+        /* Don't call set_macro_sequence here, as it can exit.
+           --warn-macro-sequence sets optarg to NULL (which uses the
+           default regexp); --warn-macro-sequence= sets optarg to ""
+           (which disables these warnings).  */
+       macro_sequence = optarg;
        break;
 
       case VERSION_OPTION:
@@ -575,7 +576,7 @@ main (int argc, char *const *argv, char *const *envp)
 
   /* Do the basic initializations.  */
   if (debugfile && !debug_set_output (debugfile))
-    M4ERROR ((0, errno, "cannot set debug file `%s'", debugfile));
+    M4ERROR ((warning_status, errno, "cannot set debug file `%s'", debugfile));
 
   input_init ();
   output_init ();
@@ -633,7 +634,13 @@ main (int argc, char *const *argv, char *const *envp)
 
        case '\1':
          seen_file = true;
-          process_file (defines->arg);
+         process_file (defines->arg);
+         break;
+
+       case DEBUGFILE_OPTION:
+         if (!debug_set_output (defines->arg))
+           M4ERROR ((warning_status, errno, "cannot set debug file `%s'",
+                     debugfile ? debugfile : "<stderr>"));
          break;
 
        default:
-- 
1.6.0.4