Re: RFA: utils.c change

[Andrew Cagney <ac131313 at cygnus dot com>]

Kevin Buettner wrote:
> 
> 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.)


FYI,

That wasn't Davids code but rather mine.  Your change looks good to me.

	enjoy,
		Andrew


PS: The following obscure tweek is possible.  It gets IN to dump its
contents directly onto STREAM avoiding several mallocs/copies.  Given I
distilled the below from some libgdb code, it strongly suggests that I
should generalize this and a standard interface.  I'd suggest checking
it in and I'll go throug later.

static void
do_write (void *data, const char *buffer, long length_buffer)
{
  gdb_file_write (data, buffer, length_buffer);
}

void
out_put (struct gdb_file *in, struct gdb_file *stream)
{
  gdb_file_put (in, do_write, stream);
}


> 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);
>  }
>