Re: Patch for gdb/mi 604

["J. Johnston" <jjohnstn at redhat dot com>]

Elena Zannoni wrote:
> 
> 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.
> 

It's a timing issue.  The old code used to set gdb_stdout up in mi_command_loop.  By that
time, the initial message has already been issued and not "consolized".
Because I was attempting not to disturb the mi1 interface, I have to put
the code in the right spot for mi2 and above, and leave it in the wrong spot
for mi1 and below.  Both places have to make the check for the mi level.


> 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.
>

I had this originally, but I figured that the tests should flag an error
if the wrong type of message is issued for either mi setting.  Simply
placing them in the untested bucket would probably get them ignored.
 
>  > +            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 $" {