From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32446 invoked by alias); 31 Jan 2011 19:00:34 -0000 Received: (qmail 32329 invoked by uid 22791); 31 Jan 2011 19:00:32 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from e24smtp04.br.ibm.com (HELO e24smtp04.br.ibm.com) (32.104.18.25) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 31 Jan 2011 19:00:27 +0000 Received: from /spool/local by e24smtp04.br.ibm.com with XMail ESMTP for from ; Mon, 31 Jan 2011 17:00:23 -0200 Received: from mailhub3.br.ibm.com ([9.18.232.110]) by e24smtp04.br.ibm.com ([10.172.0.140]) with XMail ESMTP; Mon, 31 Jan 2011 17:00:21 -0200 Received: from d24av01.br.ibm.com (d24av01.br.ibm.com [9.8.31.91]) by mailhub3.br.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p0VJ76Hd2679034 for ; Mon, 31 Jan 2011 17:07:06 -0200 Received: from d24av01.br.ibm.com (loopback [127.0.0.1]) by d24av01.br.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p0VIwqwJ025144 for ; Mon, 31 Jan 2011 16:58:52 -0200 Received: from [9.8.13.137] ([9.8.13.137]) by d24av01.br.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with ESMTP id p0VIwq8W025129; Mon, 31 Jan 2011 16:58:52 -0200 Subject: Re: [RFA] Implement support for PowerPC BookE ranged breakpoints From: Thiago Jung Bauermann To: Eli Zaretskii Cc: gdb-patches@sourceware.org In-Reply-To: <83bp31idgm.fsf@gnu.org> References: <1296177985.2843.82.camel@hactar> <83bp31idgm.fsf@gnu.org> Content-Type: text/plain; charset="UTF-8" Date: Mon, 31 Jan 2011 19:07:00 -0000 Message-ID: <1296500344.3120.10.camel@hactar> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit x-cbid: 11013119-8936-0000-0000-0000004FBD43 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: 2011-01/txt/msg00591.txt.bz2 On Fri, 2011-01-28 at 11:28 +0200, Eli Zaretskii wrote: > > From: Thiago Jung Bauermann > > Date: Thu, 27 Jan 2011 23:26:25 -0200 > Thanks. Thanks for the review. > > +* When locally debugging programs on PowerPC BookE processors running > ^^^^^^^ > You mean "natively", right? I don't think we use "local debugging" > elsewhere in our documentation, do we? Oops. Fixed. > > + a Linux kernel version 2.6.34 or later, GDB supports ranged breakpoints, > > + which stop execution of the inferior whenever it executes any address > > + within the specified range. See the "PowerPC Embedded" section in the > > "whenever it executes an instruction at any address within the > specified range". (You cannot "execute" an address.) It was an analogy with "read an address" and "write an address", but I guess even those expressions are not that good. :-) Fixed. > > + if (b->disposition == disp_del) > > + printf_filtered (_("Temporary ranged breakpoint")); > > + else > > + printf_filtered (_("Ranged breakpoint")); > > + printf_filtered (_(" %d"), b->number); > > This snippet violates one of the rules of translation-ready software: > don't construct phrases from parts. I suggest to make " %d" part of > each of the two possibilities. Agreed. Fixed. > > +static void > > +print_recreate_ranged_breakpoint (struct breakpoint *b, struct ui_file *fp) > > +{ > > + fprintf_unfiltered (fp, "break-range %s", b->exp_string); > > Should this string be in _() ? No. This function is called by the "save breakpoints" command an is expected to write to fp a GDB command which will recreate the breakpoint. > > + sals_start.sals = (struct symtab_and_line *) > > + xmalloc (sizeof (struct symtab_and_line)); > > + sals_start.nelts = 1; > > Spaces and TABs mixup alert! Hum, I couldn't find the mixup. the lines above and below xmalloc need to use spaces because they start at column 7. The xmalloc line uses TAB since it starts at column 9 (and it doesn't have any spaces only one TAB). > > +The breakpoint will stop execution of the inferior whenever it\n\ > > +executes any address within the [start-address, end-address] range\n\ > ^^^^^^^^^^^^^^^^^^^^ > Same correction as in NEWS. Fixed. > > +PowerPC embedded processors support hardware accelerated ranged breakpoints. > > +A @dfn{ranged breakpoint} stops execution of the inferior whenever it > > Use @dfn when you first introduce a term, in this case in the previous > sentence. Ok. > Also, please add here a @cindex entry about "ranged breakpoint". Done. > > +executes any address within the range it specifies. To set a ranged > ^^^^^^^^^^^^^^^^^^^^ > Same correction as in NEWS. Fixed. > > +Set a breakpoint for an address range. > > +@var{start-location} and @var{end-location} can specify a function name, > > +a line number, an offset of lines from the current line or from the start > > +location, or an address of an instruction (@xref{Specify Location}, > > You want a "see @ref here", not @xref. The latter will produce "See" > with a capital S, which will look like a typo. In general, @xref is > supposed to use only at the beginning of a sentence. Fixed. > > +The breakpoint will stop execution of the inferior whenever it > > +executes any address within the specified range, (including > ^^^^^^^^^^^^^^^^^^^^ > Same correction as above. Fixed. > The patch for NEWS and the manual is okay with those changes. Great! The changes will show up next time I post the patch, after the code review. -- []'s Thiago Jung Bauermann IBM Linux Technology Center