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.