RFA: "show user" output

RFA: "show user" output

[Jackie Smith Cashion]

I recently noticed that the "show user" output is messed up. The "if",
"else", "while", and "end" keywords are printed twice on the line. I
have attached a patch that fixes this.

This is the first RFA I've submitted. I'll probably need some pointers
on the approval and check in process.
-- 

Jackie Smith Cashion

email:  jsmith@redhat.com
phone:  931 438 2432
Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.1590
diff -c -3 -p -r1.1590 ChangeLog
*** ChangeLog	2001/09/05 23:44:43	1.1590
--- ChangeLog	2001/09/06 21:36:28
***************
*** 1,3 ****
--- 1,8 ----
+ 2001-09-06  Jackie Smith Cashion  <jsmith@redhat.com>
+ 
+ 	* cli/cli-script.c (print_command_lines): Remove extra
+ 	"if", "else", "while", and "end" from show user output.
+ 
  2001-09-05  Elena Zannoni  <ezannoni@redhat.com>
  
          * gdbarch.sh: Move include of dis-asm.h so it is generated earlier
Index: cli/cli-script.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-script.c,v
retrieving revision 1.7
diff -c -3 -p -r1.7 cli-script.c
*** cli-script.c	2001/06/17 15:16:12	1.7
--- cli-script.c	2001/09/06 21:36:32
*************** print_command_lines (struct ui_out *uiou
*** 171,181 ****
        /* A while command.  Recursively print its subcommands and continue.  */
        if (list->control_type == while_control)
  	{
- 	  ui_out_text (uiout, "while ");
  	  ui_out_field_fmt (uiout, NULL, "while %s", list->line);
  	  ui_out_text (uiout, "\n");
  	  print_command_lines (uiout, *list->body_list, depth + 1);
- 	  ui_out_field_string (uiout, NULL, "end");
  	  if (depth)
  	    ui_out_spaces (uiout, 2 * depth);
  	  ui_out_text (uiout, "end\n");
--- 171,179 ----
*************** print_command_lines (struct ui_out *uiou
*** 186,192 ****
        /* An if command.  Recursively print both arms before continueing.  */
        if (list->control_type == if_control)
  	{
- 	  ui_out_text (uiout, "if ");
  	  ui_out_field_fmt (uiout, NULL, "if %s", list->line);
  	  ui_out_text (uiout, "\n");
  	  /* The true arm. */
--- 184,189 ----
*************** print_command_lines (struct ui_out *uiou
*** 197,208 ****
  	    {
  	      if (depth)
  		ui_out_spaces (uiout, 2 * depth);
- 	      ui_out_field_string (uiout, NULL, "else");
  	      ui_out_text (uiout, "else\n");
  	      print_command_lines (uiout, list->body_list[1], depth + 1);
  	    }
  
- 	  ui_out_field_string (uiout, NULL, "end");
  	  if (depth)
  	    ui_out_spaces (uiout, 2 * depth);
  	  ui_out_text (uiout, "end\n");
--- 194,203 ----
From kevinb@cygnus.com Thu Sep 06 16:21:00 2001
From: Kevin Buettner <kevinb@cygnus.com>
To: gdb-patches@sources.redhat.com
Cc: "H . J . Lu" <hjl@lucon.org>
Subject: Re: [PATCH RFA] partial-stab.h patch amendment
Date: Thu, 06 Sep 2001 16:21:00 -0000
Message-id: <1010906232048.ZM8395@ocotillo.lan>
References: <1010905224331.ZM6026@ocotillo.lan> <npheugkyqo.fsf@zwingli.cygnus.com> <1010906205537.ZM8109@ocotillo.lan> <kevinb@cygnus.com>
X-SW-Source: 2001-09/msg00081.html
Content-length: 5795

I recently committed the following changes:

> 	* dbxread.c (process_one_symbol): Don't use error result from
> 	find_stab_function_addr().
> 	* partial-stab.h (case 'F'): Likewise.
> 
> 	* partial-stab.h (case 'f'): Make SOFUN_ADDRESS_MAYBE_MISSING
> 	code match that used for case 'F'.  This fixes the divergence
> 	that was introduced by my 1999-09-14 changes to partial-stab.h.

Those of you who've been following this thread should also be aware of
some additional analysis from H.J. Lu:

    http://sources.redhat.com/ml/gdb/2001-09/msg00046.html

I have dumped GDB's minimal symbol table and the partial symbol table
and have found that __strtol_internal appears once in the minimal
symbol table:

[413] T 0x40062634 __strtol_internal section .text  frame-dwarf2.c

But it appears twice in the partial symbol table:

Partial symtab for source file eval.c (object 0x828cb74)

  Read from object file /lib/libdl.so.2 (0x8270ff8)
  Relocate symbols by 0x4002d000, 0x4002d000, 0x4002d000, 0x4002d000, 0x4002d000, 0x4002d000, 0x4002d000, 0x4002d000, 0x4002d000, 0x4002d000, 0x4002d000, 0x4002d000, 0x4002d000, 0x4002d000, 0x4002d000, 0x4002d000, 0x4002d000, 0x4002d000, 0x4002d000, 0x4002d000, 0x4002d000, 0x4002d000, 0x0, 0x4002d000, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0
  Symbols cover text addresses 0x4002eb20-0x4002f10a
  Depends on 2 other partial symtabs.
    0 0x8283714 dlopen.c
    1 0x82844b8 dlerror.c
  Global partial symbols:
    `__strtol_internal', function, 0x4002d000
...

Partial symtab for source file ../sysdeps/generic/strtol.c (object 0x837860c)

  Read from object file /lib/i686/libc.so.6 (0x828d130)
  Full symtab was read (at 0x852f800 by function at 0x80f805c)
  Relocate symbols by 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x40031000, 0x0, 0x40031000, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0
  Symbols cover text addresses 0x40062630-0x40062d3f
  Depends on 12 other partial symtabs.
    0 0x402fb3cc soinit.c
    1 0x83558d8 iconv_open.c
    2 0x82a08bc ../sysdeps/unix/sysv/linux/init-first.c
    3 0x8355284 ../sysdeps/generic/check_fds.c
    4 0x8357534 gconv.c
    5 0x8353024 ../sysdeps/generic/libc-start.c
    6 0x8362750 setlocale.c
    7 0x835be40 gconv_trans.c
    8 0x8357a18 gconv_db.c
    9 0x8364744 mb_cur_max.c
    10 0x83650bc lc-time.c
    11 0x835b220 gconv_simple.c
  Global partial symbols:
    `__strtol_internal', function, 0x40062634
...

The first of the psymtab entries for __strtol_internal is wrong.  When
I disassemble at the address in question, I see:

    (gdb) x/8i 0x4002d000
    0x4002d000:     jg     0x4002d047
    0x4002d002:     dec    %esp
    0x4002d003:     inc    %esi
    0x4002d004:     add    %eax,(%ecx)
    0x4002d006:     add    %eax,(%eax)
    0x4002d008:     add    %al,(%eax)
    0x4002d00a:     add    %al,(%eax)
    0x4002d00c:     add    %al,(%eax)

...which doesn't look like a function prologue to me.  (I've disassembled
the second one and it *does* look like a function prologue.)

So, how did this happen?

If you read H.J.'s message, you'll see that __strtol_internal appears
in libdl.so.2 as a weak undefined symbol.  It got that way presumably
through a bug somewhere else in the toolchain (linker perhaps?) which
converted it from a weak defined symbol to a weak undefined symbol.
Yet, in the process, the stabs entries related to this symbol weren't
removed.  Thus we end up having stabs entries which describe an
undefined symbol.

Prior to my patch, when gdb read such an entry (N_FUN, descriptor 'F',
with zero value), it would (assuming that the SOFUN_ADDRESS_MAYBE_MISSING
code is enabled):

    1) Notice that the value is zero.

    2) Due to the zero value, it would attempt to look up the value
       using the minimal symbols.

    3) Set pst->textlow to this value if it hadn't already been set.

For the particular case involving __strtol_internal, this symbol
appeared first and that caused the symbol's value to be set to
zero AND it caused pst->textlow to also be set to zero.  (It was this
last bit which caused disasterous consequences when looking up other
symbols.)

Now, with my patch applied, gdb no longer attempts to use a zero
value from the result of looking up the minimal symbol since this
(usually) represents an error condition.  Instead, the original
value is used which will be zero plus the offset of the .text
section.  This at least causes pst->textlow to be set correctly -
or at least closer to where it ought to be.

Either with or without my recent patch, a symbol ends up getting
defined which shouldn't be.  I'm not sure that this is of any great
concern because GDB gets its symbol information from several sources
and if one of these sources is inconsistent with another, it is not
unreasonable to expect GDB to become somewhat confused.  Also, it does
seem reasonable to expect there to be times when a symbol will be
defined by the debug info, but not by the linkage info.  So, it doesn't
seem like a good idea to refuse to add symbols to the partial symbol
table just because they don't exist in the minimal symbol table.

I don't really expect any comments on this message, but I wanted
write down what I know of the matter before moving on to something
else...

Kevin

Re: RFA: "show user" output

[Fernando Nasser]

Jackie Smith Cashion wrote:
> 
> I recently noticed that the "show user" output is messed up. The "if",
> "else", "while", and "end" keywords are printed twice on the line. I
> have attached a patch that fixes this.
> 

Have you checked the MI output before and after your patch?


In particular, this bit suggests that in this last chunk you've removed
the 
"good" else/end and left the "bad" ones ;-)
 
> *************** print_command_lines (struct ui_out *uiou
> *** 197,208 ****
>             {
>               if (depth)
>                 ui_out_spaces (uiout, 2 * depth);
> -             ui_out_field_string (uiout, NULL, "else");
>               ui_out_text (uiout, "else\n");
>               print_command_lines (uiout, list->body_list[1], depth + 1);
>             }
> 
> -         ui_out_field_string (uiout, NULL, "end");
>           if (depth)
>             ui_out_spaces (uiout, 2 * depth);
>           ui_out_text (uiout, "end\n");
> --- 194,203 ----

Regards,
Fernando

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Re: RFA: "show user" output

[Jackie Smith Cashion]

The output looks okay when I run the modified GDB. I must admit it's not
clear to me which ui_out function to use for which purpose. Here's how I
made my decisions:

while: I removed the call to ui_out_text since it does not output the
rest of the "while" command line.

while end: It is important to output the indenting spaces before the
keyword, so I removed the call to ui_out_field_string that came before
the ui_out_spaces.

if: I removed the call to ui_out_text since it does not output the rest
of the "if" command line.

else: Since ui_out_text had been used above to output "end" which like
"else" has no command line arguments, I kept the call to ui_out_text
here.

if end: I removed the call to ui_out_field_string that came before the
ui_out_spaces.

I guess the question is whether ui_out_text or ui_out_field_string
should be used to print a simple string with no additional arguments. Is
there a general rule? 

Jackie


Fernando Nasser wrote:
> 
> Have you checked the MI output before and after your patch?
> 
> In particular, this bit suggests that in this last chunk you've removed
> the
> "good" else/end and left the "bad" ones ;-)
> 
> > *************** print_command_lines (struct ui_out *uiou
> > *** 197,208 ****
> >             {
> >               if (depth)
> >                 ui_out_spaces (uiout, 2 * depth);
> > -             ui_out_field_string (uiout, NULL, "else");
> >               ui_out_text (uiout, "else\n");
> >               print_command_lines (uiout, list->body_list[1], depth + 1);
> >             }
> >
> > -         ui_out_field_string (uiout, NULL, "end");
> >           if (depth)
> >             ui_out_spaces (uiout, 2 * depth);
> >           ui_out_text (uiout, "end\n");
> > --- 194,203 ----
> 
> Regards,
> Fernando
> 
> --
> Fernando Nasser
> Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
> 2323 Yonge Street, Suite #300
> Toronto, Ontario   M4P 2C9

-- 

Jackie Smith Cashion

email:  jsmith@redhat.com
phone:  931 438 2432

Re: RFA: "show user" output

[Fernando Nasser]

Jackie Smith Cashion wrote:
>
> (...)
> 
> Is
> there a general rule?
> 

Yes.

uiout_text() only generates formatting output so things look good for
human 
eyes.  The MI incarnation of it produces no output (other machine
oriented 
implementations would do the same).  ui_out_spaces() is just a
specialized
version of uiout_text().

uiout_field_*() functions produce output that conveys information.  The
"cli_"
implementation of output prints them while the "mi_" version formats
them for
machine decoding.

So, if your console output is not looking good, you adjust your
uiout_text()
or uiout_spaces() so it looks nice.  Avoid meddling with the
uiout_field_*()
calls.

P.S.: I believe there is a test case for the MI output of commands in
the
gdb/testsuite/gdb.mi subdirectory.

Regards,
Fernando

> 
> Fernando Nasser wrote:
> >
> > Have you checked the MI output before and after your patch?
> >
> > In particular, this bit suggests that in this last chunk you've removed
> > the
> > "good" else/end and left the "bad" ones ;-)
> >
> > > *************** print_command_lines (struct ui_out *uiou
> > > *** 197,208 ****
> > >             {
> > >               if (depth)
> > >                 ui_out_spaces (uiout, 2 * depth);
> > > -             ui_out_field_string (uiout, NULL, "else");
> > >               ui_out_text (uiout, "else\n");
> > >               print_command_lines (uiout, list->body_list[1], depth + 1);
> > >             }
> > >
> > > -         ui_out_field_string (uiout, NULL, "end");
> > >           if (depth)
> > >             ui_out_spaces (uiout, 2 * depth);
> > >           ui_out_text (uiout, "end\n");
> > > --- 194,203 ----
> >
> > Regards,
> > Fernando
> >
> > --
> > Fernando Nasser
> > Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
> > 2323 Yonge Street, Suite #300
> > Toronto, Ontario   M4P 2C9
> 
> --
> 
> Jackie Smith Cashion
> 
> email:  jsmith@redhat.com
> phone:  931 438 2432

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Re: RFA: "show user" output

[Jackie Smith Cashion]

Thanks for the ui_out explanation. This means I need to use
ui_out_spaces to do the indenting and ui_out_field_fmt and
ui_out_field_string to print the keywords. Following the example set in
the current version of the code I will use ui_out_text to print the
carriage returns. Here's my modified patch:

--
2001-09-08  Jackie Smith Cashion  <jsmith@redhat.com>

	* cli/cli-script.c (print_command_lines): Remove extra
	"if", "else", "while", and "end" from show user output.

--
Index: cli-script.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-script.c,v
retrieving revision 1.7
diff -c -3 -p -r1.7 cli-script.c
*** cli-script.c	2001/06/17 15:16:12	1.7
--- cli-script.c	2001/09/09 01:56:47
*************** print_command_lines (struct ui_out *uiou
*** 171,184 ****
        /* A while command.  Recursively print its subcommands and
continue.  */
        if (list->control_type == while_control)
  	{
- 	  ui_out_text (uiout, "while ");
  	  ui_out_field_fmt (uiout, NULL, "while %s", list->line);
  	  ui_out_text (uiout, "\n");
  	  print_command_lines (uiout, *list->body_list, depth + 1);
- 	  ui_out_field_string (uiout, NULL, "end");
  	  if (depth)
  	    ui_out_spaces (uiout, 2 * depth);
! 	  ui_out_text (uiout, "end\n");
  	  list = list->next;
  	  continue;
  	}
--- 171,183 ----
        /* A while command.  Recursively print its subcommands and
continue.  */
        if (list->control_type == while_control)
  	{
  	  ui_out_field_fmt (uiout, NULL, "while %s", list->line);
  	  ui_out_text (uiout, "\n");
  	  print_command_lines (uiout, *list->body_list, depth + 1);
  	  if (depth)
  	    ui_out_spaces (uiout, 2 * depth);
! 	  ui_out_field_string (uiout, NULL, "end");
! 	  ui_out_text (uiout, "\n");
  	  list = list->next;
  	  continue;
  	}
*************** print_command_lines (struct ui_out *uiou
*** 186,192 ****
        /* An if command.  Recursively print both arms before
continueing.  */
        if (list->control_type == if_control)
  	{
- 	  ui_out_text (uiout, "if ");
  	  ui_out_field_fmt (uiout, NULL, "if %s", list->line);
  	  ui_out_text (uiout, "\n");
  	  /* The true arm. */
--- 185,190 ----
*************** print_command_lines (struct ui_out *uiou
*** 198,211 ****
  	      if (depth)
  		ui_out_spaces (uiout, 2 * depth);
  	      ui_out_field_string (uiout, NULL, "else");
! 	      ui_out_text (uiout, "else\n");
  	      print_command_lines (uiout, list->body_list[1], depth + 1);
  	    }
  
- 	  ui_out_field_string (uiout, NULL, "end");
  	  if (depth)
  	    ui_out_spaces (uiout, 2 * depth);
! 	  ui_out_text (uiout, "end\n");
  	  list = list->next;
  	  continue;
  	}
--- 196,209 ----
  	      if (depth)
  		ui_out_spaces (uiout, 2 * depth);
  	      ui_out_field_string (uiout, NULL, "else");
! 	      ui_out_text (uiout, "\n");
  	      print_command_lines (uiout, list->body_list[1], depth + 1);
  	    }
  
  	  if (depth)
  	    ui_out_spaces (uiout, 2 * depth);
! 	  ui_out_field_string (uiout, NULL, "end");
! 	  ui_out_text (uiout, "\n");
  	  list = list->next;
  	  continue;
  	}

-- 
Jackie Smith Cashion

email:  jsmith@redhat.com
phone:  931 438 2432

Re: RFA: "show user" output

[Fernando Nasser]

Jackie Smith Cashion wrote:
> 
> Thanks for the ui_out explanation. This means I need to use
> ui_out_spaces to do the indenting and ui_out_field_fmt and
> ui_out_field_string to print the keywords. Following the example set in
> the current version of the code I will use ui_out_text to print the
> carriage returns. Here's my modified patch:
> 

You've got it.  From visual inspection it looks perfect.

I have only one question: how come there wasn't a FAILing test in
gdb.base/commands.exp (or there was)?  And in gdb.mi?

Tradition says that as you've fixed it you should also add a little
test to prevent it to be broken again ;-)

Thanks for the fix.

Regards,
Fernando



> --
> 2001-09-08  Jackie Smith Cashion  <jsmith@redhat.com>
> 
>         * cli/cli-script.c (print_command_lines): Remove extra
>         "if", "else", "while", and "end" from show user output.
> 
> --
> Index: cli-script.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/cli/cli-script.c,v
> retrieving revision 1.7
> diff -c -3 -p -r1.7 cli-script.c
> *** cli-script.c        2001/06/17 15:16:12     1.7
> --- cli-script.c        2001/09/09 01:56:47
> *************** print_command_lines (struct ui_out *uiou
> *** 171,184 ****
>         /* A while command.  Recursively print its subcommands and
> continue.  */
>         if (list->control_type == while_control)
>         {
> -         ui_out_text (uiout, "while ");
>           ui_out_field_fmt (uiout, NULL, "while %s", list->line);
>           ui_out_text (uiout, "\n");
>           print_command_lines (uiout, *list->body_list, depth + 1);
> -         ui_out_field_string (uiout, NULL, "end");
>           if (depth)
>             ui_out_spaces (uiout, 2 * depth);
> !         ui_out_text (uiout, "end\n");
>           list = list->next;
>           continue;
>         }
> --- 171,183 ----
>         /* A while command.  Recursively print its subcommands and
> continue.  */
>         if (list->control_type == while_control)
>         {
>           ui_out_field_fmt (uiout, NULL, "while %s", list->line);
>           ui_out_text (uiout, "\n");
>           print_command_lines (uiout, *list->body_list, depth + 1);
>           if (depth)
>             ui_out_spaces (uiout, 2 * depth);
> !         ui_out_field_string (uiout, NULL, "end");
> !         ui_out_text (uiout, "\n");
>           list = list->next;
>           continue;
>         }
> *************** print_command_lines (struct ui_out *uiou
> *** 186,192 ****
>         /* An if command.  Recursively print both arms before
> continueing.  */
>         if (list->control_type == if_control)
>         {
> -         ui_out_text (uiout, "if ");
>           ui_out_field_fmt (uiout, NULL, "if %s", list->line);
>           ui_out_text (uiout, "\n");
>           /* The true arm. */
> --- 185,190 ----
> *************** print_command_lines (struct ui_out *uiou
> *** 198,211 ****
>               if (depth)
>                 ui_out_spaces (uiout, 2 * depth);
>               ui_out_field_string (uiout, NULL, "else");
> !             ui_out_text (uiout, "else\n");
>               print_command_lines (uiout, list->body_list[1], depth + 1);
>             }
> 
> -         ui_out_field_string (uiout, NULL, "end");
>           if (depth)
>             ui_out_spaces (uiout, 2 * depth);
> !         ui_out_text (uiout, "end\n");
>           list = list->next;
>           continue;
>         }
> --- 196,209 ----
>               if (depth)
>                 ui_out_spaces (uiout, 2 * depth);
>               ui_out_field_string (uiout, NULL, "else");
> !             ui_out_text (uiout, "\n");
>               print_command_lines (uiout, list->body_list[1], depth + 1);
>             }
> 
>           if (depth)
>             ui_out_spaces (uiout, 2 * depth);
> !         ui_out_field_string (uiout, NULL, "end");
> !         ui_out_text (uiout, "\n");
>           list = list->next;
>           continue;
>         }
> 
> --
> Jackie Smith Cashion
> 
> email:  jsmith@redhat.com
> phone:  931 438 2432

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Re: RFA: "show user" output

[Jackie Smith Cashion]

Fernando Nasser wrote:
> 
> I have only one question: how come there wasn't a FAILing test in
> gdb.base/commands.exp (or there was)?  And in gdb.mi?

There is the following test in commands.exp:

    send_gdb "define mycommand\n"
      ...
    gdb_test "while \$arg0 > 0\nset \$arg0 -= 1\nif \(\$arg0 % 2\) ==
1\np/x 0xdeadbeef\nelse\np/x 0xfeedface\nend\nend\nend" \
      ...
    gdb_test "mycommand \$foo" \
      ...
    gdb_test "show user mycommand" \
	   "while.*set.*if.*p/x.*else.*p/x.*end.*" \
	   "display user command in user_defined_command_test"

Obviously, I've cut out several lines from the actual test. But if you
look at the last call to gdb_test you see that it simply checks to see
that the macro keywords appear in the output. The test doesn't look for
proper "whitespace". This test also would not detect our current problem
where the keywords are printed twice.

> Tradition says that as you've fixed it you should also add a little
> test to prevent it to be broken again ;-)

At this time I know nearly nothing about expect scripts. I don't mind
learning about them. To help me along does anyone want to offer a
suggestion about testing for "whitespace" and extra keyword output in
this test?

Jackie Smith Cashion

email:  jsmith@redhat.com
phone:  931 438 2432

Re: RFA: "show user" output

[Andrew Cagney]

> Jackie Smith Cashion wrote:

> I have only one question: how come there wasn't a FAILing test in
> gdb.base/commands.exp (or there was)?  And in gdb.mi?

i dont think the mi side is tested.  the ``commands'' command suggests a 
cli and round 0 of the mi didn't have a cli.

there should be a pr recording the lack of this feature/test.

andrew

Re: RFA: "show user" output

[Fernando Nasser]

Jackie,

gdb_test <command to send> <regular expression of the expected output>
<test id string>

$2 is just a regular expression, just write/modify it as it was some
regexp for grep
or something and it will probably work.

Just make sure that, if you add a new test, the id string is unique
(i.e., no the
same as other tests in the same file).

Regards,
Fernando


Jackie Smith Cashion wrote:
> 
> Fernando Nasser wrote:
> >
> > I have only one question: how come there wasn't a FAILing test in
> > gdb.base/commands.exp (or there was)?  And in gdb.mi?
> 
> There is the following test in commands.exp:
> 
>     send_gdb "define mycommand\n"
>       ...
>     gdb_test "while \$arg0 > 0\nset \$arg0 -= 1\nif \(\$arg0 % 2\) ==
> 1\np/x 0xdeadbeef\nelse\np/x 0xfeedface\nend\nend\nend" \
>       ...
>     gdb_test "mycommand \$foo" \
>       ...
>     gdb_test "show user mycommand" \
>            "while.*set.*if.*p/x.*else.*p/x.*end.*" \
>            "display user command in user_defined_command_test"
> 
> Obviously, I've cut out several lines from the actual test. But if you
> look at the last call to gdb_test you see that it simply checks to see
> that the macro keywords appear in the output. The test doesn't look for
> proper "whitespace". This test also would not detect our current problem
> where the keywords are printed twice.
> 
> > Tradition says that as you've fixed it you should also add a little
> > test to prevent it to be broken again ;-)
> 
> At this time I know nearly nothing about expect scripts. I don't mind
> learning about them. To help me along does anyone want to offer a
> suggestion about testing for "whitespace" and extra keyword output in
> this test?
> 
> Jackie Smith Cashion
> 
> email:  jsmith@redhat.com
> phone:  931 438 2432

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Re: RFA: "show user" output

[Fernando Nasser]

Andrew Cagney wrote:
> 
> > Jackie Smith Cashion wrote:
> 
> > I have only one question: how come there wasn't a FAILing test in
> > gdb.base/commands.exp (or there was)?  And in gdb.mi?
> 
> i dont think the mi side is tested.  the ``commands'' command suggests a
> cli and round 0 of the mi didn't have a cli.
> 

Right.


> there should be a pr recording the lack of this feature/test.
> 

Jackie is trying to modify the current cli test to check for a
regression
in the duplicate tokens problem.

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Re: RFA: "show user" output

[Andrew Cagney]

> there should be a pr recording the lack of this feature/test.
>> 
> 
> 
> Jackie is trying to modify the current cli test to check for a
> regression
> in the duplicate tokens problem.


yes, i'm thinking of the mi side.  right now, adding an mi test wouldn't 
make sense, but recording the problem does.  we need to review / check 
the mi behavour so that we dont break it.

as can only be expected, the patches broke undocumented and untested mi 
behavour.

andrew