Re: Patch for gdb/mi 604

Elena Zannoni
2002-11-07T23:30:55Z

J. Johnston writes:
> Andrew Cagney wrote:
> >
> > > J. Johnston writes:
> > > > Elena Zannoni wrote:
> > > > >
> > > > > J. Johnston writes:
> > > > > > The following patches mi-main.c to turn on the console output prefix prior
> > > > > > to outputting the gdb startup messages. The code checks for mi level
> > > > > > and defers to old behavior for mi1.
> > > > > >
> > > > > > Ok to commit?
> > > > > >
> > > > >
> > > > > Wouldn't this affect the testsuite as well?
> > > > > Or is this behavior not tested?
> > > > >
> > > > > Elena
> > > > >
> > > >
> > > > It does not affect the testsuite. The testsuite startup code looks for
> > > > the gdb prompt or some form of error indication, but does not specifically
> > > > look at the start-up message.
> > > >
> > >
> > > Hmm, I wonder whether now it should. Since we are expecting a specific
> > > behavior it might as well be tested. Thoughts?
> >
> > It _needs_ to be tested - if it isn't tested it doesn't work :-)
> >
> > Andrew
>
> On that note, I have provided here a patch to the mi-support.exp file in
> gdb/testsuite/lib. It checks the startup message for mi and mi1 in
> the mi startup routine.
>
> I have also reincluded the code patch. Please let me know if this is ok to
> check in.
>
> -- Jeff J.
>
> gdb/testsuite/ChangeLog:
>
> 2002-11-06 Jeff Johnston <jjohnstn@redhat.com>
>
> * lib/mi-support.exp (mi_gdb_start): Verify the startup message for mi1 and
> current mi is in correct format. New mi startup message should be in
> console format. This is part of fix for PR gdb/604.
>
> gdb/mi/ChangeLog:
>
> 2002-11-06 Jeff Johnston <jjohnstn@redhat.com>
>
> * mi-main.c (mi_command_loop): Initialize raw_stdout and gdb_stdout only if
> mi version is <= 1.
> (mi_init_ui): Initialize raw_stdout and gdb_stdout if mi version is > 1 so
> startup message is treated as console output. This is part of fix for
> PR gdb/604.Index: mi-main.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/mi/mi-main.c,v
> retrieving revision 1.35
> diff -u -r1.35 mi-main.c
> --- mi-main.c 23 Oct 2002 21:17:51 -0000 1.35
> +++ mi-main.c 7 Nov 2002 01:04:06 -0000
> @@ -1464,12 +1464,16 @@
> static void
> mi_command_loop (int mi_version)
> {
> - /* HACK: Force stdout/stderr to point at the console. This avoids
> - any potential side effects caused by legacy code that is still
> - using the TUI / fputs_unfiltered_hook */
> - raw_stdout = stdio_fileopen (stdout);
> - /* Route normal output through the MIx */
> - gdb_stdout = mi_console_file_new (raw_stdout, "~");
> + if (mi_version <= 1)
> + {
> + /* HACK: Force stdout/stderr to point at the console. This avoids
> + any potential side effects caused by legacy code that is still
> + using the TUI / fputs_unfiltered_hook */
> + raw_stdout = stdio_fileopen (stdout);
> + /* Route normal output through the MIx */
> + gdb_stdout = mi_console_file_new (raw_stdout, "~");
> + }
> +
> /* Route error and log output through the MI */
> gdb_stderr = mi_console_file_new (raw_stdout, "&");
> gdb_stdlog = gdb_stderr;

I don't understand why the code that handles this is in 2 different
places depending on the interpreter version. I must be missing
something, mi_init_ui is called with -i=mi2 and with -i=mi1.

Elena

> @@ -1541,8 +1545,16 @@
> static void
> mi_init_ui (char *arg0)
> {
> - /* Eventually this will contain code that takes control of the
> - console. */
> + if (strlen (interpreter_p) <= 2 ||
> + interpreter_p[2] > '1')
> + {
> + /* HACK: Force stdout/stderr to point at the console. This avoids
> + any potential side effects caused by legacy code that is still
> + using the TUI / fputs_unfiltered_hook */
> + raw_stdout = stdio_fileopen (stdout);
> + /* Route normal output through the MIx */
> + gdb_stdout = mi_console_file_new (raw_stdout, "~");
> + }
> }
>
> void
> Index: lib/mi-support.exp
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/lib/mi-support.exp,v
> retrieving revision 1.17
> diff -u -r1.17 mi-support.exp
> --- lib/mi-support.exp 10 Sep 2002 22:28:19 -0000 1.17
> +++ lib/mi-support.exp 7 Nov 2002 01:14:58 -0000
> @@ -123,7 +123,20 @@
> return 1;
> }
> gdb_expect {
> - -re ".*$mi_gdb_prompt$" {
> + -re "~\"GNU.*\r\n~\".*$mi_gdb_prompt$" {
> + if { $MIFLAGS == "-i=mi1" } {
> + perror "(mi startup) Got unexpected new mi prompt."

I am wondering if it should use 'untested' instead of 'perror'. That's
what the other cases do.

> + remote_close host;
> + return -1;
> + }
> + verbose "GDB initialized."
> + }
> + -re "\[^~\].*$mi_gdb_prompt$" {
> + if { $MIFLAGS != "-i=mi1" } {
> + perror "(mi startup) Got unexpected old mi prompt."
> + remote_close host;
> + return -1;
> + }
> verbose "GDB initialized."
> }
> -re ".*$gdb_prompt $" {