This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC] breakpoints/19474 [was Re: RFC: branching for GDB 7.11 soon? (possibly Wed)]
- From: Pedro Alves <palves at redhat dot com>
- To: Keith Seitz <keiths at redhat dot com>, "gdb-patches at sourceware dot org ml" <gdb-patches at sourceware dot org>
- Cc: Yao Qi <yao at codesourcery dot com>
- Date: Thu, 04 Feb 2016 12:31:33 +0000
- Subject: Re: [RFC] breakpoints/19474 [was Re: RFC: branching for GDB 7.11 soon? (possibly Wed)]
- Authentication-results: sourceware.org; auth=none
- References: <20160201030638 dot GG4008 at adacore dot com> <56AFB750 dot 3030702 at redhat dot com> <56B29F17 dot 6020603 at redhat dot com>
On 02/04/2016 12:45 AM, Keith Seitz wrote:
> At Pedro's urging, I found another, simpler solution. The big problem
> with just about all solutions I was attempting to implement is
> [p]symtab_to_fululname/find_and_open_source/openp. These functions are
> just completely unaware of what the caller is attempting to do.
Thanks!
> I'm sure there are corner cases and a whole bunch of other problems with
> this approach, but at least it is isolated to one place (for better or
> worse).
Things to watch out for, out of the blue:
- relative SYMTAB_DIRNAMEs -- is there such a thing?
- symlinks. Say, what happens if foo/f.c is a symlink to f.c (perhaps
each is compiled differently, with -DWHATNOT, for example).
> +static VEC (symtab_ptr) *
> +filter_default_symtabs (const char *fullname, VEC (symtab_ptr) *symtabs)
> +{
> + int ix;
> + struct symtab *symtab;
> + VEC (symtab_ptr) *filtered_symtabs = NULL;
> + struct cleanup *back_to = make_cleanup (null_cleanup, NULL);
I think filtered_symtabs should be guarded with a cleanup as well.
> +
> + /* Iterate through the symtabs, searching for matches to FULLNAME. */
> + for (ix = 0; VEC_iterate (symtab_ptr, symtabs, ix, symtab); ++ix)
> + {
> + const char *basename = lbasename (fullname);
> + char *symtab_with_dir;
> +
> + if (SYMTAB_DIRNAME (symtab) == NULL)
> + continue;
> +
> + symtab_with_dir = concat (SYMTAB_DIRNAME (symtab), SLASH_STRING,
> + basename, NULL);
> + make_cleanup (xfree, symtab_with_dir);
> + if (streq (fullname, symtab_with_dir))
> + VEC_safe_push (symtab_ptr, filtered_symtabs, symtab);
> + else
> + {
> + /* Now try any path substitution rules. */
> + symtab_with_dir = rewrite_source_path (symtab_with_dir);
> + if (symtab_with_dir != NULL)
> + {
> + make_cleanup (xfree, symtab_with_dir);
> + if (streq (fullname, symtab_with_dir))
> + VEC_safe_push (symtab_ptr, filtered_symtabs, symtab);
> + }
> + }
This creates two cleanups per iteration here. Only two for the whole
loop would suffice, if you used free_current_contents instead of xfree.
> + }
> +
> + /* If we found no matches, use whatever symtabs were originally
> + "collected." */
> + if (filtered_symtabs == NULL)
Pedantically, checking VEC_empty instead would be better, as an initial
VEC_reserve (symtab_ptr, filtered_symtabs, VEC_size (symtab_ptr, symtab));
would make a NULL check wrong. That reserve might make sense if most of
the time we'll match all symtabs.
> + {
> + /* Found no exact matches -- use the original list. */
> + filtered_symtabs = symtabs;
> + }
> + else
> + VEC_free (symtab_ptr, symtabs);
> +
> + do_cleanups (back_to);
> + return filtered_symtabs;
> +}
Thanks,
Pedro Alves