This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On 10/17/2012 07:16 PM, Jason Richards wrote:
> Please consider this patch, which fixes -symbol-list-lines in situations
> where a symbol table is only partially loaded and situations where more
> than one symbol table corresponds to a given file.

Unfortunately, the patch is bounced back, as it does a lot of unnecessary
(and unfortunately wrong, according to the GNU coding conventions)
reformatting.

Could you please send a more complete rationale on the change?
Maybe it gets a bit more obvious once the patch only includes the necessary
changes, but it's always good to include a more complete description of
what is wrong, and how the fix attempts to fix it.

Also, is this something that could have a regression test for?

No comments on the substance, just some minor comments below on some C mechanics:

> +    ALL_PSPACE_SYMTABS (pspace, objfile, s) {
> +        if (strlen(filename) && strlen(s->filename) &&
> +                !strcmp(lbasename(filename), lbasename(s->filename))) {

- No need to compute the whole string's length to check whether it is empty.
- strcmp's result is not a boolean.

Thus:

       if (*filename != '\0' && *s->filename != '\0'
           && strcmp (lbasename (filename), lbasename (s->filename)) == 0)
         {

Also, it seems like we could cache lbasename(filename) instead of computing it
at each iteration.

-- 
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]