This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [RFA]: Fix for pending breakpoints in manually loaded/unloaded shlibs
On Wed, Aug 11, 2004 at 04:12:07PM -0400, Jeff Johnston wrote:
> Ok?
Sorry to keep picking nits; while we discuss the issue of the new
observer I went over the patch again for minor problems.
> +/* Disable any breakpoints that are in in an unloaded shared library. Only
> + apply to enabled breakpoints, disabled ones can just stay disabled. */
> +
> +void
> +disable_breakpoints_in_unloaded_shlib (struct so_list *solib)
> +{
> + struct breakpoint *b;
> + int disabled_shlib_breaks = 0;
> +
> + /* See also: insert_breakpoints, under DISABLE_UNSETTABLE_BREAK. */
Two spaces after a period, please.
> + ALL_BREAKPOINTS (b)
> + {
> +#if defined (PC_SOLIB)
I think someone pointed out that you've #ifdef'd out the entire body of
this loop. Might as well include the whole loop. The #ifdef is nasty,
but that's a preexisting problem.
> + if ((b->loc->loc_type == bp_loc_hardware_breakpoint
> + || b->loc->loc_type == bp_loc_software_breakpoint)
> + && breakpoint_enabled (b)
> + && !b->loc->duplicate)
> + {
> + char *so_name = PC_SOLIB (b->loc->address);
While I'm ranting about preexisting problems, it would be nice if
PC_SOLIB returned the solib, instead of just its name... but enh.
> + if (so_name
> + && !strcmp (so_name, solib->so_name))
> + {
> + b->enable_state = bp_shlib_disabled;
> + /* At this point, we cannot rely on remove_breakpoint
> + succeeding so we must mark the breakpoint as not inserted
> + to prevent future errors occurring in remove_breakpoints. */
> + b->loc->inserted = 0;
> + if (!disabled_shlib_breaks)
> + {
> + target_terminal_ours_for_output ();
> + warning ("Temporarily disabling unloaded shared library breakpoints:");
> + }
> + disabled_shlib_breaks = 1;
> + warning ("breakpoint #%d ", b->number);
I think you're missing a space after the colon, in the first warning.
Also, this use of multiple warning() statements is neither i18n
friendly nor MI/GUI friendly - you may get a separate dialog box for
each. I believe other places do this with sprintf; still not 100% i18n
friendly, but avoids the MI/GUI problems. I can't find an example
offhand.
--
Daniel Jacobowitz