From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3430 invoked by alias); 8 Feb 2008 01:37:40 -0000 Received: (qmail 3421 invoked by uid 22791); 8 Feb 2008 01:37:39 -0000 X-Spam-Check-By: sourceware.org Received: from viper.snap.net.nz (HELO viper.snap.net.nz) (202.37.101.8) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 08 Feb 2008 01:37:08 +0000 Received: from kahikatea.snap.net.nz (181.31.255.123.static.snap.net.nz [123.255.31.181]) by viper.snap.net.nz (Postfix) with ESMTP id 1C4043D9F9B; Fri, 8 Feb 2008 14:37:05 +1300 (NZDT) Received: by kahikatea.snap.net.nz (Postfix, from userid 1000) id 5C14D8FC6D; Fri, 8 Feb 2008 14:36:55 +1300 (NZDT) From: Nick Roberts MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <18347.45621.630420.453287@kahikatea.snap.net.nz> Date: Fri, 08 Feb 2008 01:37:00 -0000 To: Joel Brobecker Cc: ghost@cs.msu.su, gdb-patches@sourceware.org Subject: Re: (gdb-6.8) Discard breakpoint address if shared library is unloaded In-Reply-To: <20080207063817.GA3907@adacore.com> References: <20080204214226.GF20922@adacore.com> <18343.35253.667689.989145@kahikatea.snap.net.nz> <20080205001213.GJ21614@adacore.com> <18343.45995.719252.980359@kahikatea.snap.net.nz> <20080207063817.GA3907@adacore.com> X-Mailer: VM 7.19 under Emacs 22.1.90.1 X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-02/txt/msg00145.txt.bz2 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", ""); > > else if (header_of_multiple) > > ui_out_field_string (uiout, "addr", ""); > > + else if (loc->shlib_disabled) > > + ui_out_field_string (uiout, "addr", ""); > > 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", ""); > - else if (header_of_multiple) > + if (header_of_multiple) > ui_out_field_string (uiout, "addr", ""); > + if (b->loc == NULL || loc->shlib_disabled) > + ui_out_field_string (uiout, "addr", ""); > 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