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

[Nick Roberts <nickrob at snap dot net dot nz>]

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 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.

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.

-- 
Nick                                           http://www.inet.net.nz/~nickrob