Stage 24c: Improve display of macro names with embedded NUL.

* m4/m4module.h (m4_push_string_init): Add parameters.
* m4/m4private.h (struct m4_macro_args): Remove argv0 and
argv0_len, now redundant with info.
(m4__push_wrapup_init): Add parameter.
* m4/input.c (m4_push_string_init, m4__push_wrapup_init): Track
location from caller, rather than context.
(composite_peek, composite_read, match_input): Adjust callers.
* m4/utility.c (m4_symbol_value_lookup): Quote macro name.
(m4_verror_at_line): Allow NUL in macro name.
* m4/macro.c (trace_flush, m4_trace_prepare, trace_pre): Allow NUL
in trace.
(expand_macro): No longer munge context location.
(collect_arguments, m4_arg_text, m4_arg_empty, m4_arg_len)
(m4_make_argv_ref, m4_push_arg, m4_wrap_args): Adjust callers.
* modules/gnu.c (builtin, indir): Likewise.
* tests/null.m4: Enhance test.
* tests/null.err: Adjust expected output.
* tests/null.out: Likewise.

Signed-off-by: Eric Blake <ebb9@byu.net>

11 files changed, 117 insertions, 71 deletions

diff --git a/ChangeLog b/ChangeLog
index e9c86073..f06aee74 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,30 @@
 2008-06-02  Eric Blake  <ebb9@byu.net>
 
+	Stage 24c: Improve display of macro names with embedded NUL.
+	Quote instances of problematic characters in macro names when
+	presented to user.  Track location via call_info rather than
+	munging context for every expansion.
+	Memory impact: none.
+	Speed impact: slight penalty, due to more bookkeeping.
+	* m4/m4module.h (m4_push_string_init): Add parameters.
+	* m4/m4private.h (struct m4_macro_args): Remove argv0 and
+	argv0_len, now redundant with info.
+	(m4__push_wrapup_init): Add parameter.
+	* m4/input.c (m4_push_string_init, m4__push_wrapup_init): Track
+	location from caller, rather than context.
+	(composite_peek, composite_read, match_input): Adjust callers.
+	* m4/utility.c (m4_symbol_value_lookup): Quote macro name.
+	(m4_verror_at_line): Allow NUL in macro name.
+	* m4/macro.c (trace_flush, m4_trace_prepare, trace_pre): Allow NUL
+	in trace.
+	(expand_macro): No longer munge context location.
+	(collect_arguments, m4_arg_text, m4_arg_empty, m4_arg_len)
+	(m4_make_argv_ref, m4_push_arg, m4_wrap_args): Adjust callers.
+	* modules/gnu.c (builtin, indir): Likewise.
+	* tests/null.m4: Enhance test.
+	* tests/null.err: Adjust expected output.
+	* tests/null.out: Likewise.
+
 	Stage 24b: Allow embedded NUL in macro names.
 	Use length rather than NUL-termination when tracking macro names.
 	All APIs dealing with symbol table changed.
diff --git a/m4/input.c b/m4/input.c
index d2a508e5..212c1c64 100644
--- a/m4/input.c
+++ b/m4/input.c
@@ -438,10 +438,12 @@ string_print (m4_input_block *me, m4 *context, m4_obstack *obs,
 }
 
 /* First half of m4_push_string ().  The pointer next points to the
-   new input_block.  Return the obstack that will collect the
-   expansion text.  */
+   new input_block.  FILE and LINE describe the location where the
+   macro starts that is generating the expansion (even if the location
+   has advanced in the meantime).  Return the obstack that will
+   collect the expansion text.  */
 m4_obstack *
-m4_push_string_init (m4 *context)
+m4_push_string_init (m4 *context, const char *file, int line)
 {
   /* Free any memory occupied by completely parsed input.  */
   assert (!next);
@@ -450,8 +452,8 @@ m4_push_string_init (m4 *context)
   /* Reserve the next location on the obstack.  */
   next = (m4_input_block *) obstack_alloc (current_input, sizeof *next);
   next->funcs = &string_funcs;
-  next->file = m4_get_current_file (context);
-  next->line = m4_get_current_line (context);
+  next->file = file;
+  next->line = line;
   next->u.u_s.len = 0;
 
   return current_input;
@@ -691,7 +693,7 @@ composite_peek (m4_input_block *me, m4 *context, bool allow_argv)
 	  /* Rather than directly parse argv here, we push another
 	     input block containing the next unparsed argument from
 	     argv.  */
-	  m4_push_string_init (context);
+	  m4_push_string_init (context, me->file, me->line);
 	  m4__push_arg_quote (context, current_input, chain->u.u_a.argv,
 			      chain->u.u_a.index,
 			      m4__quote_cache (M4SYNTAX, NULL,
@@ -759,7 +761,7 @@ composite_read (m4_input_block *me, m4 *context, bool allow_quote,
 	  /* Rather than directly parse argv here, we push another
 	     input block containing the next unparsed argument from
 	     argv.  */
-	  m4_push_string_init (context);
+	  m4_push_string_init (context, me->file, me->line);
 	  m4__push_arg_quote (context, current_input, chain->u.u_a.argv,
 			      chain->u.u_a.index,
 			      m4__quote_cache (M4SYNTAX, NULL,
@@ -1012,10 +1014,11 @@ m4_input_print (m4 *context, m4_obstack *obs, int debug_level)
 
 /* Return an obstack ready for direct expansion of wrapup text, and
    set *END to the location that should be updated if any builtin
-   tokens are wrapped.  This should be followed by
-   m4__push_wrapup_finish ().  */
+   tokens are wrapped.  Store the location of CALLER with the wrapped
+   text.  This should be followed by m4__push_wrapup_finish ().  */
 m4_obstack *
-m4__push_wrapup_init (m4 *context, m4__symbol_chain ***end)
+m4__push_wrapup_init (m4 *context, const m4_call_info *caller,
+		      m4__symbol_chain ***end)
 {
   m4_input_block *i;
   m4__symbol_chain *chain;
@@ -1032,8 +1035,8 @@ m4__push_wrapup_init (m4 *context, m4__symbol_chain ***end)
       i = (m4_input_block *) obstack_alloc (wrapup_stack, sizeof *i);
       i->prev = wsp;
       i->funcs = &composite_funcs;
-      i->file = m4_get_current_file (context);
-      i->line = m4_get_current_line (context);
+      i->file = caller->file;
+      i->line = caller->line;
       i->u.u_c.chain = i->u.u_c.end = NULL;
       wsp = i;
     }
@@ -1046,8 +1049,8 @@ m4__push_wrapup_init (m4 *context, m4__symbol_chain ***end)
   chain->next = NULL;
   chain->type = M4__CHAIN_LOC;
   chain->quote_age = 0;
-  chain->u.u_l.file = m4_get_current_file (context);
-  chain->u.u_l.line = m4_get_current_line (context);
+  chain->u.u_l.file = caller->file;
+  chain->u.u_l.line = caller->line;
   *end = &i->u.u_c.end;
   return wrapup_stack;
 }
@@ -1396,7 +1399,8 @@ match_input (m4 *context, const char *s, bool consume)
     }
 
   /* Failed or shouldn't consume, push back input.  */
-  st = m4_push_string_init (context);
+  st = m4_push_string_init (context, m4_get_current_file (context),
+			    m4_get_current_line (context));
   obstack_grow (st, t, n);
   m4_push_string_finish ();
   return result;
diff --git a/m4/m4module.h b/m4/m4module.h
index 1bd90c7d..346463d3 100644
--- a/m4/m4module.h
+++ b/m4/m4module.h
@@ -503,7 +503,7 @@ extern	void	m4_skip_line	(m4 *context, const m4_call_info *);
 
 extern	void	m4_push_file	(m4 *, FILE *, const char *, bool);
 extern	void	m4_push_builtin	(m4 *, m4_obstack *, m4_symbol_value *);
-extern	m4_obstack	*m4_push_string_init	(m4 *);
+extern	m4_obstack	*m4_push_string_init	(m4 *, const char *, int);
 extern	void	m4_push_string_finish	(void);
 extern	bool	m4_pop_wrapup	(m4 *);
 extern	void	m4_input_print	(m4 *, m4_obstack *, int);
diff --git a/m4/m4private.h b/m4/m4private.h
index c092016c..e1c81631 100644
--- a/m4/m4private.h
+++ b/m4/m4private.h
@@ -307,8 +307,6 @@ struct m4_macro_args
   bool_bitfield flatten : 1;
   /* True if any token contains builtins.  */
   bool_bitfield has_func : 1;
-  const char *argv0; /* The macro name being expanded.  */
-  size_t argv0_len; /* Length of argv0.  */
   /* The value of quote_age for all tokens, or 0 if quote_age changed
      during parsing or any token is potentially unsafe and requires a
      rescan.  */
@@ -561,7 +559,8 @@ extern	void		m4__append_builtin (m4_obstack *, const m4__builtin *,
 					    m4__symbol_chain **);
 extern	bool		m4__push_symbol (m4 *, m4_symbol_value *, size_t,
 					 bool);
-extern	m4_obstack	*m4__push_wrapup_init (m4 *, m4__symbol_chain ***);
+extern	m4_obstack	*m4__push_wrapup_init (m4 *, const m4_call_info *,
+					       m4__symbol_chain ***);
 extern	void		m4__push_wrapup_finish (void);
 extern	m4__token_type	m4__next_token (m4 *, m4_symbol_value *, int *,
 					m4_obstack *, bool,
diff --git a/m4/macro.c b/m4/macro.c
index f28b3f69..1436ea9a 100644
--- a/m4/macro.c
+++ b/m4/macro.c
@@ -443,10 +443,6 @@ expand_macro (m4 *context, const char *name, size_t len, m4_symbol *symbol)
   m4__macro_arg_stacks *stack;	/* Storage for this macro.  */
   m4_call_info info;		/* Context of this macro call.  */
 
-  /* TODO - use m4_call_info to avoid temporary munging of global state.  */
-  const char *loc_close_file;
-  int loc_close_line;
-
   /* Obstack preparation.  */
   level = context->expansion_level;
   if (context->stacks_count <= level)
@@ -506,17 +502,11 @@ recursion limit of %zu exceeded, use -L<N> to change it"),
   args_scratch = obstack_finish (stack->args);
 
   /* The actual macro call.  */
-  loc_close_file = m4_get_current_file (context);
-  loc_close_line = m4_get_current_line (context);
-  m4_set_current_file (context, info.file);
-  m4_set_current_line (context, info.line);
-  expansion = m4_push_string_init (context);
+  expansion = m4_push_string_init (context, info.file, info.line);
   m4_macro_call (context, value, expansion, argv);
   m4_push_string_finish ();
 
   /* Cleanup.  */
-  m4_set_current_file (context, loc_close_file);
-  m4_set_current_line (context, loc_close_line);
   argv->info = NULL;
 
   --context->expansion_level;
@@ -538,7 +528,8 @@ recursion limit of %zu exceeded, use -L<N> to change it"),
 	  if (debug_macro_level & PRINT_ARGCOUNT_CHANGES)
 	    xfprintf (stderr, "m4debug: -%d- `%s' in use, level=%d, "
 		      "refcount=%zu, argcount=%zu\n", info.call_id,
-		      argv->argv0, level, stack->refcount, stack->argcount);
+		      argv->info->name, level, stack->refcount,
+		      stack->argcount);
 	}
       else
 	{
@@ -571,14 +562,12 @@ collect_arguments (m4 *context, m4_call_info *info, m4_symbol *symbol,
   args.has_func = false;
   /* Must copy here, since we are consuming tokens, and since symbol
      table can be changed during argument collection.  */
-  args.argv0 = (char *) obstack_copy0 (arguments, info->name, info->name_len);
-  args.argv0_len = info->name_len;
+  info->name = (char *) obstack_copy0 (arguments, info->name, info->name_len);
   args.quote_age = m4__quote_age (M4SYNTAX);
   args.info = info;
   args.level = context->expansion_level - 1;
   args.arraylen = 0;
   obstack_grow (argv_stack, &args, offsetof (m4_macro_args, array));
-  info->name = args.argv0;
 
   if (m4__next_token_is_open (context))
     {
@@ -890,16 +879,17 @@ static void
 trace_flush (m4 *context, unsigned int start)
 {
   char *str;
+  size_t len = obstack_object_size (&context->trace_messages);
   FILE *file = m4_get_debug_file (context);
 
   if (file)
     {
-      obstack_1grow (&context->trace_messages, '\0');
+      /* TODO - quote nonprintable characters if debug is tty?  */
       str = (char *) obstack_base (&context->trace_messages);
-      fprintf (file, "%s\n", &str[start]);
+      fwrite (&str[start], 1, len - start, file);
+      fputc ('\n', file);
     }
-  start -= obstack_object_size (&context->trace_messages);
-  obstack_blank (&context->trace_messages, start);
+  obstack_blank (&context->trace_messages, start - len);
 }
 
 /* Do pre-argument-collection tracing for the macro described in INFO.
@@ -917,7 +907,8 @@ m4_trace_prepare (m4 *context, const m4_call_info *info,
   if (info->trace && (info->debug_level & M4_DEBUG_TRACE_CALL))
     {
       unsigned int start = trace_header (context, info);
-      trace_format (context, "%s ... = ", info->name);
+      obstack_grow (&context->trace_messages, info->name, info->name_len);
+      obstack_grow (&context->trace_messages, " ... = ", 7);
       m4__symbol_value_print (context, value, &context->trace_messages, quotes,
 			      false, NULL, &arg_length, module);
       trace_flush (context, start);
@@ -935,7 +926,8 @@ trace_pre (m4 *context, m4_macro_args *argv)
   unsigned int start = trace_header (context, argv->info);
 
   assert (argv->info->trace);
-  trace_format (context, "%s", M4ARG (0));
+  obstack_grow (&context->trace_messages, argv->info->name,
+		argv->info->name_len);
 
   if (1 < m4_arg_argc (argv) && (trace_level & M4_DEBUG_TRACE_ARGS))
     {
@@ -1258,7 +1250,10 @@ m4_arg_text (m4 *context, m4_macro_args *argv, size_t arg, bool flatten)
   m4_obstack *obs;
 
   if (arg == 0)
-    return argv->argv0;
+    {
+      assert (argv->info);
+      return argv->info->name;
+    }
   if (argv->argc <= arg)
     return "";
   value = arg_symbol (argv, arg, NULL, flatten);
@@ -1453,8 +1448,12 @@ m4_arg_equal (m4 *context, m4_macro_args *argv, size_t indexa, size_t indexb)
 bool
 m4_arg_empty (m4_macro_args *argv, size_t arg)
 {
-  return (arg ? m4_arg_symbol (argv, arg) == &empty_symbol
-	  : !argv->argv0_len);
+  if (!arg)
+    {
+      assert (argv->info);
+      return !argv->info->name_len;
+    }
+  return m4_arg_symbol (argv, arg) == &empty_symbol;
 }
 
 /* Given ARGV, return the length of argument ARG.  Abort if the
@@ -1467,7 +1466,10 @@ m4_arg_len (m4 *context, m4_macro_args *argv, size_t arg)
   size_t len;
 
   if (arg == 0)
-    return argv->argv0_len;
+    {
+      assert (argv->info);
+      return argv->info->name_len;
+    }
   if (argv->argc <= arg)
     return 0;
   value = m4_arg_symbol (argv, arg);
@@ -1629,8 +1631,6 @@ m4_make_argv_ref (m4 *context, m4_macro_args *argv, const char *argv0,
     }
   new_argv->argc = argv->argc - 1;
   new_argv->inuse = false;
-  new_argv->argv0 = argv0;
-  new_argv->argv0_len = argv0_len;
   new_argv->quote_age = argv->quote_age;
   new_argv->info = info;
   info->trace = (argv->info->debug_level & M4_DEBUG_TRACE_ALL) || trace;
@@ -1649,7 +1649,9 @@ m4_push_arg (m4 *context, m4_obstack *obs, m4_macro_args *argv, size_t arg)
 
   if (arg == 0)
     {
-      m4_set_symbol_value_text (&value, argv->argv0, argv->argv0_len, 0);
+      assert (argv->info);
+      m4_set_symbol_value_text (&value, argv->info->name, argv->info->name_len,
+				0);
       if (m4__push_symbol (context, &value, context->expansion_level - 1,
 			   argv->inuse))
 	arg_mark (argv);
@@ -1721,7 +1723,7 @@ m4_wrap_args (m4 *context, m4_macro_args *argv)
   if (limit == 2 && m4_arg_empty (argv, 1))
     return;
 
-  obs = m4__push_wrapup_init (context, &end);
+  obs = m4__push_wrapup_init (context, argv->info, &end);
   for (i = 1; i < limit; i++)
     {
       if (i != 1)
diff --git a/m4/utility.c b/m4/utility.c
index 7c911912..c104779d 100644
--- a/m4/utility.c
+++ b/m4/utility.c
@@ -132,9 +132,11 @@ m4_symbol_value_lookup (m4 *context, m4_macro_args *argv, size_t i,
   if (m4_is_arg_text (argv, i))
     {
       const char *name = M4ARG (i);
-      result = m4_symbol_lookup (M4SYMTAB, name, M4ARGLEN (i));
+      size_t len = M4ARGLEN (i);
+      result = m4_symbol_lookup (M4SYMTAB, name, len);
       if (must_exist && !result)
-	m4_warn (context, 0, argv->info, _("undefined macro `%s'"), name);
+	m4_warn (context, 0, argv->info, _("undefined macro %s"),
+		 quotearg_style_mem (locale_quoting_style, name, len));
     }
   else
     m4_warn (context, 0, argv->info, _("invalid macro name ignored"));
@@ -154,6 +156,7 @@ m4_verror_at_line (m4 *context, bool warn, int status, int errnum,
   char *full = NULL;
   char *safe_macro = NULL;
   const char *macro = caller ? caller->name : NULL;
+  size_t len = caller ? caller->name_len : 0;
   const char *file = caller ? caller->file : m4_get_current_file (context);
   int line = caller ? caller->line : m4_get_current_line (context);
 
@@ -161,29 +164,33 @@ m4_verror_at_line (m4 *context, bool warn, int status, int errnum,
   /* Sanitize MACRO, since we are turning around and using it in a
      format string.  The allocation is overly conservative, but
      problematic macro names only occur via indir or changesyntax.  */
-  if (macro && strchr (macro, '%'))
+  if (macro && memchr (macro, '%', len))
     {
-      char *p = safe_macro = xcharalloc (2 * strlen (macro) + 1);
-      do
+      char *p = safe_macro = xcharalloc (2 * len);
+      const char *end = macro + len;
+      while (macro != end)
 	{
 	  if (*macro == '%')
-	    *p++ = '%';
+	    {
+	      *p++ = '%';
+	      len++;
+	    }
 	  *p++ = *macro++;
 	}
-      while (*macro);
-      *p = '\0';
     }
+  if (macro)
+    /* Use slot 1, so that the rest of the code can use the simpler
+       quotearg interface in slot 0.  */
+    macro = quotearg_n_mem (1, safe_macro ? safe_macro : macro, len);
   /* Prepend warning and the macro name, as needed.  But if that fails
      for non-memory reasons (unlikely), then still use the original
      format.  */
   if (warn && macro)
-    full = xasprintf (_("Warning: %s: %s"),
-		      quotearg (safe_macro ? safe_macro : macro), format);
+    full = xasprintf (_("Warning: %s: %s"), macro, format);
   else if (warn)
     full = xasprintf (_("Warning: %s"), format);
   else if (macro)
-    full = xasprintf (_("%s: %s"),
-		      quotearg (safe_macro ? safe_macro : macro), format);
+    full = xasprintf (_("%s: %s"), macro, format);
   verror_at_line (status, errnum, line ? file : NULL, line,
 		  full ? full : format, args);
   free (full);
diff --git a/modules/gnu.c b/modules/gnu.c
index a13ae110..ead1b62b 100644
--- a/modules/gnu.c
+++ b/modules/gnu.c
@@ -29,6 +29,7 @@
 #endif
 
 #include "modules/m4.h"
+#include "quotearg.h"
 
 /* Rename exported symbols for dlpreload()ing.  */
 #define m4_builtin_table	gnu_LTX_m4_builtin_table
@@ -404,7 +405,8 @@ M4BUILTIN_HANDLER (builtin)
 {
   const m4_call_info *me = m4_arg_info (argv);
   const char *name;
-  m4_symbol_value *value;
+  size_t len;
+  m4_symbol_value *value = NULL;
 
   if (!m4_is_arg_text (argv, 1))
     {
@@ -419,9 +421,12 @@ M4BUILTIN_HANDLER (builtin)
 	      return;
 	    }
 	  name = M4ARG (2);
-	  value = m4_builtin_find_by_name (NULL, name);
+	  len = M4ARGLEN (2);
+	  if (len == strlen (name))
+	    value = m4_builtin_find_by_name (NULL, name);
 	  if (value == NULL)
-	    m4_warn (context, 0, me, _("undefined builtin `%s'"), name);
+	    m4_warn (context, 0, me, _("undefined builtin %s"),
+		     quotearg_style_mem (locale_quoting_style, name, len));
 	  else
 	    {
 	      m4_push_builtin (context, obs, value);
@@ -434,16 +439,19 @@ M4BUILTIN_HANDLER (builtin)
   else
     {
       name = M4ARG (1);
-      value = m4_builtin_find_by_name (NULL, name);
+      len = M4ARGLEN (1);
+      if (len == strlen (name))
+	value = m4_builtin_find_by_name (NULL, name);
       if (value == NULL)
-	m4_warn (context, 0, me, _("undefined builtin `%s'"), name);
+	m4_warn (context, 0, me, _("undefined builtin %s"),
+		 quotearg_style_mem (locale_quoting_style, name, len));
       else
 	{
 	  const m4_builtin *bp = m4_get_symbol_value_builtin (value);
 	  m4_macro_args *new_argv;
 	  bool flatten = (bp->flags & M4_BUILTIN_FLATTEN_ARGS) != 0;
-	  new_argv = m4_make_argv_ref (context, argv, name, M4ARGLEN (1),
-				       flatten, false);
+	  new_argv = m4_make_argv_ref (context, argv, name, len, flatten,
+				       false);
 	  if (!m4_bad_argc (context, argc - 1, m4_arg_info (new_argv),
 			    bp->min_args, bp->max_args,
 			    (bp->flags & M4_BUILTIN_SIDE_EFFECT) != 0))
@@ -678,7 +686,8 @@ M4BUILTIN_HANDLER (indir)
       m4_symbol *symbol = m4_symbol_lookup (M4SYMTAB, name, len);
 
       if (symbol == NULL)
-	m4_warn (context, 0, me, _("undefined macro `%s'"), name);
+	m4_warn (context, 0, me, _("undefined macro %s"),
+		 quotearg_style_mem (locale_quoting_style, name, len));
       else
 	{
 	  m4_macro_args *new_argv;
diff --git a/tests/null.err b/tests/null.err
index d8258182..8bf1f4f7 100644
--- a/tests/null.err
+++ b/tests/null.err
diff --git a/tests/null.m4 b/tests/null.m4
index 851d665e..55bd3bd8 100644
--- a/tests/null.m4
+++ b/tests/null.m4
diff --git a/tests/null.out b/tests/null.out
index 03f919b4..3a96faa3 100644
--- a/tests/null.out
+++ b/tests/null.out
diff --git a/tests/others.at b/tests/others.at
index 34680442..781e07ac 100644
--- a/tests/others.at
+++ b/tests/others.at
@@ -351,11 +351,11 @@ AT_SETUP([nul character])
 
 # We don't embed null.* in here, since it is harder to guarantee the
 # behavior of NUL through autom4te.
-cp "$abs_srcdir/null.out" expout
-cp "$abs_srcdir/null.err" experr
+sed "s|null.m4|$abs_srcdir/null.m4|" < "$abs_srcdir/null.out" > expout
+sed "s|null.m4|$abs_srcdir/null.m4|" < "$abs_srcdir/null.err" > experr
 
 dnl all but m4exit
-AT_CHECK_M4([-Dm4exit "$abs_srcdir/null.m4"], [0], [expout], [experr])
+AT_CHECK_M4([-Dm4exit -I "$abs_srcdir" null.m4], [0], [expout], [experr])
 
 dnl just m4exit
 AT_CHECK_M4(["$abs_srcdir/null.m4"], [2],