This is the mail archive of the
gdb-patches@sourceware.cygnus.com
mailing list for the GDB project.
Re: RFA: utils.c change
- To: Kevin Buettner <kevinb at cygnus dot com>
- Subject: Re: RFA: utils.c change
- From: Andrew Cagney <ac131313 at cygnus dot com>
- Date: Thu, 25 Nov 1999 10:53:44 +1100
- CC: David Taylor <taylor at cygnus dot com>, gdb-patches at sourceware dot cygnus dot com
- Organization: Cygnus Solutions
- References: <991124225117.ZM12897@ocotillo.lan>
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);
> }
>