Re: (gdb-6.8) Discard breakpoint address if shared library is unloaded

[Vladimir Prus <ghost@cs.msu.su>]

On Friday 08 February 2008 04:36:53 Nick Roberts wrote:
> Joel Brobecker writes:
>  > Vladimir,
>  > 
>  > I could use a little help with your feedback. I think I'm getting it,
>  > but I think you might know more about this part of the code.
> 
> Since it looks like Vladimir can't find the time to look at this

Huh? I don't think expecting a response within one business day
is quite reasonable.

> and because 
> you might like to make the branch shortly, FWIW, I've looked more closely
> at it:
> 
>  > I think we're still not there, unfortunately.
>  > 
>  > >   	 state -- it will be apparent from the locations.  */
>  > >         if (header_of_multiple)
>  > >   	pending = 0;
>  > > !       ui_out_field_fmt (uiout, "enabled", "%c", 
>  > > !  			bpenables[(int) b->enable_state]);
>  > >       }
>  > 
>  > The following code above "ui_out_field_fmt" can be deleted now:
>  > 
>  >       int pending = (b->loc == NULL || b->loc->shlib_disabled);
>  >       /* For header of multiple, there's no point showing pending
>  >          state -- it will be apparent from the locations.  */
>  >       if (header_of_multiple)
>  >         pending = 0;
>  > 
>  > That means that the curly braces can go too. We also need to restore
>  > the spacing after the enable state. We end up with:
> 
> I agree.
> 
>  >   annotate_field (3);
>  >   if (part_of_multiple)
>  >     ui_out_field_string (uiout, "enabled", loc->enabled ? "y" : "n");
>  >   else
>  >     ui_out_field_fmt (uiout, "enabled", "%c",
>  >                       bpenables[(int) b->enable_state]);
>  >   ui_out_spaces (uiout, 2);
> 
> Putting this back to 2 spaces requires a similar change to the header to keep
> alignment:
> 
> *************** breakpoint_1 (int bnum, int allflag)
> *** 3780,3786 ****
>     ui_out_table_header (uiout, 4, ui_left, "disp", "Disp");		/* 3 */
>     if (nr_printable_breakpoints > 0)
>       annotate_field (3);
> !   ui_out_table_header (uiout, 4, ui_left, "enabled", "Enb");	/* 4 */
>     if (addressprint)
>   	{
>   	  if (nr_printable_breakpoints > 0)
> --- 3767,3773 ----
>     ui_out_table_header (uiout, 4, ui_left, "disp", "Disp");		/* 3 */
>     if (nr_printable_breakpoints > 0)
>       annotate_field (3);
> !   ui_out_table_header (uiout, 3, ui_left, "enabled", "Enb");	/* 4 */
>     if (addressprint)
>   	{
>   	  if (nr_printable_breakpoints > 0)
> 
> 
>  > >   	      ui_out_field_string (uiout, "addr", "<PENDING>");
>  > >   	    else if (header_of_multiple)
>  > >   	      ui_out_field_string (uiout, "addr", "<MULTIPLE>");
>  > > + 	    else if (loc->shlib_disabled)
>  > > + 	      ui_out_field_string (uiout, "addr", "<PENDING>");
>  > >   	    else
>  > >   	      ui_out_field_core_addr (uiout, "addr", loc->address);
>  > >   	  }
>  > 
>  > I understand what Vladimir said, but I think that the following should
>  > work too:
>  > 
>  > @@ -3552,10 +3552,10 @@ print_one_breakpoint_location (struct br
>  >  	if (addressprint)
>  >  	  {
>  >  	    annotate_field (4);
>  > -	    if (b->loc == NULL)
>  > -	      ui_out_field_string (uiout, "addr", "<PENDING>");
>  > -	    else if (header_of_multiple)
>  > +	    if (header_of_multiple)
>  >  	      ui_out_field_string (uiout, "addr", "<MULTIPLE>");
>  > +	    if (b->loc == NULL || loc->shlib_disabled)
>  > +	      ui_out_field_string (uiout, "addr", "<PENDING>");
>  >  	    else
>  >  	      ui_out_field_core_addr (uiout, "addr", loc->address);
>  >  	  }
>  > 
>  > The idea is that you can't have header_of_multiple=1 and b->loc == NULL
>  > at the same time. So it's OK to move the check for header_of_multiple
>  > up.  And that avoids the code duplication.
> 
> I agree.
> 
> I can confirm that Gdb works as I would expect with these changes.

Since you seem to have a patch relative to previous one, would you mind
sending a complete patch relative to CVS HEAD?

> 
> On a related note, breakpoint.c now has 7 columns:
> 
>   ui_out_table_header (uiout, 7, ui_left, "number", "Num");		/* 1 */
> 
> up from the previous 3:
> 
>   ui_out_table_header (uiout, 3, ui_left, "number", "Num");		/* 1 */
> 
> presumably for breakpoint numbers like 1.2 etc but it still seems to
> anticipate a very large number of breakpoints.

That's part of the original multiple location patch; I don't think
it's related in any way to what we're discussing now.

- Volodya