RFA: utils.c change

[Kevin Buettner <kevinb at cygnus dot com>]

David,

I was prompted to make a change to verror() in utils.c while working
on the linux/ppc gdb port.  The problem is that the va_list, args, is
passed to two different calls of vfprintf_filtered(), once to print it
to gdb_stderr, and a second time to save it as the last error.  In
each case, vfprintf_filtered() will (at some point) call va_arg()
repeatedly to scan the argument list.  Some platforms (apparently)
don't mind if you run through the arguments in a va_list more than
once.  But linux/ppc definitely does.  (It segfaults.)

I studied a number of different ways of fixing this problem:

    a)  Calling va_start to reset the va_list... but this would've
        meant changing the callers and dramatically changing the
        way verror is called.

    b)  Making a copy of the va_list argument and then using the
        copy for the second va_list traversal.  I tried to make
        this work, but it is notoriously difficult to make such
        a copy that will work everywhere.  I.e,

	    va_list args_copy;
	    memcpy (&args_copy, &args, sizeof (va_list));

        doesn't work on linux/ppc.

	If I were to restructure things to look the same as
	int_vasprintf() in libiberty, then we might have a shot at
	making it work, but I've had problems in the past with this
	function as well.  I think it's best to avoid making a copy if
	possible.

    c)  Rearrange the code so that the va_list is traversed only
	once.  I chose this option.  If we store the last error first,
	we end up with the error message in a memory buffer and it is
	easy to then use fputs_filtered() to write it out to
	gdb_stderr.

The other (minor) change that I made was to remove the extraneous
va_end() call.  According to the documentation that I have, va_end()
should be placed in the same function as the va_start().  I've
checked the two places where verror() is called, and in both
places, the code in question calls va_end() after calling verror().

Please let me know if it's okay to check in the change below...

	* utils.c (verror): Don't traverse va_list argument twice.  Also,
	removed extraneous va_end() call.

Index: utils.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/utils.c,v
retrieving revision 1.225
diff -u -p -r1.225 utils.c
--- utils.c	1999/11/16 00:03:36	1.225
+++ utils.c	1999/11/24 21:58:25
@@ -525,17 +525,28 @@ verror ()
 NORETURN void
 verror (const char *string, va_list args)
 {
+  char *err_string;
+  struct cleanup *err_string_cleanup;
   /* FIXME: cagney/1999-11-10: All error calls should come here.
      Unfortunatly some code uses the sequence: error_begin(); print
      error message; return_to_top_level.  That code should be
      flushed. */
   error_begin ();
-  vfprintf_filtered (gdb_stderr, string, args);
-  fprintf_filtered (gdb_stderr, "\n");
-  /* Save it as the last error as well (no newline) */
+  /* NOTE: It's tempting to just do the following...
+	vfprintf_filtered (gdb_stderr, string, args);
+     and then follow with a similar looking statement to cause the message
+     to also go to gdb_lasterr.  But if we do this, we'll be traversing the
+     va_list twice which works on some platforms and fails miserably on
+     others. */
+  /* Save it as the last error */
   gdb_file_rewind (gdb_lasterr);
   vfprintf_filtered (gdb_lasterr, string, args);
-  va_end (args);
+  /* Retrieve the last error and print it to gdb_stderr */
+  err_string = error_last_message ();
+  err_string_cleanup = make_cleanup (free, err_string);
+  fputs_filtered (err_string, gdb_stderr);
+  fprintf_filtered (gdb_stderr, "\n");
+  do_cleanups (err_string_cleanup);
   return_to_top_level (RETURN_ERROR);
 }