This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Fix MI -symbol-list-lines
- From: Tom Tromey <tromey at redhat dot com>
- To: Jason Richards <Jason_Richards at mentor dot com>
- Cc: Pedro Alves <palves at redhat dot com>, <gdb-patches at sourceware dot org>
- Date: Tue, 13 Nov 2012 14:37:39 -0700
- Subject: Re: [PATCH] Fix MI -symbol-list-lines
- References: <507EF5FC.6000907@mentor.com> <507EFAF0.2040606@redhat.com> <507F1895.8020802@mentor.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