[RFA]: Fix for pending breakpoints in manually loaded/unloaded shlibs

[Jeff Johnston]

The updated patch is attached.  The testcase had to be slightly modified to 
match the new warning message format.

2004-08-23  Jeff Johnston  <jjohnstn@redhat.com>

         * observer.sh: Add struct so_list declaration.
         * Makefile.in: Add dependencies on observer.h for solib.c and
         breakpoint.c.
         * breakpoint.c (disable_breakpoints_in_unloaded_shlib): New
         function.
         (_initialize_breakpoint): Register
         disable_breakpoints_in_unloaded_shlib as an observer of the
         "solib unloaded" observation event.
         (re_enable_breakpoints_in_shlibs): For bp_shlib_disabled breakpoints,
         call decode_line_1 so unfound breakpoint errors are silent.
         * solib.c (update_solib_list): When a solib is discovered to have
         been unloaded by the program, notify all observers of the
         "solib unloaded" observation event.


2004-08-23  Jeff Johnston  <jjohnstn@redhat.com>

         * gdb.base/unload.exp: Fix expected warning message to match
         latest format.


Ok to commit?

-- Jeff J.

Jeff Johnston wrote:
> Daniel Jacobowitz wrote:
> 
>> On Wed, Aug 18, 2004 at 03:22:22PM -0400, Jeff Johnston wrote:
>>
>>> Daniel Jacobowitz wrote:
>>>
>>>> On Wed, Aug 11, 2004 at 04:12:07PM -0400, Jeff Johnston wrote:
>>>>
>>>>> +    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.
>>>>
>>>
>>> What you do want to see so I don't waste my time on this.  As you 
>>> already know, this routine was copied from the routine which disables 
>>> shared library breakpoints in breakpoint.c.  Is it sufficient to just 
>>> issue the warning that I am temporarily disabling unloaded shared 
>>> library breakpoints and not spell out each breakpoint in turn?  I can 
>>> see this as really annoying and pointless to an end-user if there are 
>>> hundreds or thousands of breakpoints.
>>
>>
>>
>> That's a good idea.  How about this?
>>
>>     target_terminal_ours_for_output ();
>>     warning ("Temporarily disabling breakpoints for unloaded shared 
>> library \"%s\",
>>          so_name);
>>
> 
> Yes, that's exactly what I had in mind.  Consider it done plus your 
> other comments.  Eli, in light of what Daniel and Andrew have said 
> regarding the value of having an observer, may I repost with changes and 
> check the code in?
> 
> -- Jeff J.
> 
> 
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: unload.patch2
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20040823/66e3293a/attachment.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: unloadtest.patch
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20040823/66e3293a/attachment-0001.ksh>