Re: [PATCH] Fix MI -symbol-list-lines

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Jason" == Jason Richards <Jason_Richards@mentor.com> writes:

I didn't see a reply to this...

Jason> Thanks Pedro, sorry for being sloppy.  Here is an updated patch.
Jason> Also, if I were going to provide a test for this, it would
Jason> probably involve an executable file and a script that invokes GDB
Jason> in MI mode.  Does this sound like an appropriate test?  Are there
Jason> existing tests somewhere like this that I can use as an example?

There are many in gdb/testsuite/gdb.mi.  Your test should work like one
of these.  Testing with gcov showed that our test suite doesn't cover
mi-symbol-cmds.c at all :-(

The patch needs a ChangeLog entry as well.

Jason> +  filebase = lbasename(filename);

The GNU style has a space before the paren.  See the GNU coding
standards for more info on the details of the style.

Jason>  +  /* This creates the "demand" for a symbol table associated with
Jason> +     "filename".  Although we do not use 's', this is an important
Jason> +     step because it forces and partial symbol tables associated
Jason> +     with "filename" to be expanded into full symbol tables. */
Jason> +  find_line_symtab(s, 1, NULL, NULL);

Probably want "forces any" rather than "forces and".

Calling this for side effects seems odd to me.

Jason> +  ALL_PSPACE_SYMTABS (pspace, objfile, s) {

Wrong brace placement here.

Jason> +    if (!strcmp(filebase, lbasename(s->filename)))
Jason> +      if (LINETABLE (s) != NULL && LINETABLE (s)->nitems > 0)
Jason> +        for (i = 0; i < LINETABLE (s)->nitems; i++)
Jason> +        {
Jason> +          cleanup_tuple = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
Jason> +          ui_out_field_core_addr (uiout, "pc", gdbarch, LINETABLE (s)->item[i].pc);
Jason> +          ui_out_field_int (uiout, "line", LINETABLE (s)->item[i].line);
Jason> +          do_cleanups (cleanup_tuple);
Jason> +        }
Jason> +  }
Jason>   do_cleanups (cleanup_stack);

With this change I think you will dump the line tables of all the files
whose base name matches the argument.  But this could be a lot of
information.  And, it could repeat the lines.  I venture that this isn't
what MI clients are expecting.

I think seeing a test case would be helpful though.

Tom