From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28096 invoked by alias); 20 Mar 2012 15:42:11 -0000 Received: (qmail 28084 invoked by uid 22791); 20 Mar 2012 15:42:09 -0000 X-SWARE-Spam-Status: No, hits=-1.6 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 20 Mar 2012 15:41:55 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1SA1C6-0003h1-9C from Hui_Zhu@mentor.com ; Tue, 20 Mar 2012 08:41:54 -0700 Received: from SVR-ORW-FEM-02.mgc.mentorg.com ([147.34.96.206]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Tue, 20 Mar 2012 08:41:53 -0700 Received: from [127.0.0.1] (147.34.91.1) by svr-orw-fem-02.mgc.mentorg.com (147.34.96.168) with Microsoft SMTP Server id 14.1.289.1; Tue, 20 Mar 2012 08:41:53 -0700 Message-ID: <4F68A53F.6040103@mentor.com> Date: Tue, 20 Mar 2012 15:42:00 -0000 From: Hui Zhu User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120310 Thunderbird/11.0 MIME-Version: 1.0 To: Eli Zaretskii CC: , Subject: Re: [PATCH] Add autoload-breakpoints [7/7] autoload-breakpoints doc References: <4F6450FF.1070103@mentor.com> <83wr6j33u1.fsf@gnu.org> In-Reply-To: <83wr6j33u1.fsf@gnu.org> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit 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: 2012-03/txt/msg00741.txt.bz2 Hi Eli, Thanks for you review. On 03/17/12 18:43, Eli Zaretskii wrote: >> Date: Sat, 17 Mar 2012 16:53:19 +0800 >> From: Hui Zhu >> CC: Stan Shebs >> >> This patch add the doc for the autoload-breakpoints. > > Thanks. > >> +If the remote stub support, @value{GDBN} can get autoload-breakpoints from >> +remote stub. > > @value{GDBN} can receive the information about autoload-breakpoints > from the remote stub, if the stub supports that. > > Also, as we don't mention autoload-breakpoints anywhere else in the > manual, we need to explain here what these breakpoints are and why > controlling them is useful. What about: Some stub support set breakpoints with itself, with this function, GDB can handle the breakpoint that set by the stub better. > >> +@item set breakpoint autoload query >> +If this option is query (the default), @value{GDBN} will query to user > ^^^^^ > @samp{query} > >> +how to handle the autoload-breakpints when @value{GDBN} connect to the stub. > ^^^^^^^^^^ ^^^^^^^ > A typo. Also, "connects". > >> +@item set breakpoint autoload merge >> +If this option is merge, the autoload-breakpoints of @value{GDBN} > ^^^^^ > @samp{merge} > >> +and stub will merge together when @value{GDBN} connect to stub. > > I don't understand what it means to "merge" breakpoints. It means that merge together. Keep both the autoload-breakpoints in the GDB and the stub. > >> +@item set breakpoint autoload gdb >> +If this option is gdb, the autoload-breakpoints of stub will be removed > ^^^ > @samp{gdb} > >> +when GDB connect to stub. > ^^^ > @value{GDBN} > >> +@item set breakpoint autoload stub >> +If this option is stub, the autoload-breakpoints of GDB will be removed > ^^^^ ^^^ > @samp{stub} and @value{GDBN} > >> +@node Autoload-breakpoints Format >> +@section Autoload-breakpoints Format >> +@cindex Autoload-breakpoints Format >> + >> +@subsection Autoload-breakpoints base format > > Please don't add subsections without a node (and a menu in the parent > section). Such subsections cannot be navigated with on-line Info > commands. Should I change it to: @node Autoload-breakpoints base format @subsection Autoload-breakpoints base format @cindex Autoload-breakpoints base format > >> +Autoload-breakpoints base format describe the operation of > ^^^^^^^^ > "describes" > >> +the autoload-breakpints in @value{GDBN} and the stub. > ^^^^^^^^^^ > A typo. > > Anyway, it sounds like this describes the format of the packet, not of > autoload-breakpoints or something similar. Therefore, I think it > should be part of the packet description, not a separate section and > not in this place in the manual. > >> +@cindex @var{id}@samp{:}@var{command}@samp{:}@var{addr_string}@samp{:}@var{type}@samp{:}@var{ignore_num} packet > > This index entry is useless; please remove it. > >> +@table @samp >> +@item @var{id} >> +This is the id in hex string format of this command want to control. > > This is the number, as a hex string, of the autoload-breakpoint that > this command wants to control. > >> +@item @var{command} >> +This is command char. Include E (enable) and D (disable). > > This is the command character, either @samp{E} (for ``enable'') or > @samp{D} (for ``disable''). > >> +If this autoload-breakpoint @var{id} is not exist, create one and >> +enable or disable it. > > If the autoload-breakpoint @var{id} does not exist, create one and > enable or disable it. > >> If it is exist, item follow it will be ignore >> +and just disable and enable the autoload-breakpoints. > > If it does exist, the following items will be ignored, and the > autoload-breakpoint will be enabled or disabled as specified by > @var{command}. > >> +@item @var{addr_string} >> +If need create an autoload-breakpoint, this is the address string >> +that encoded in hex string. > > This is the address of an autoload-breakpoint to create, encoded as a > hex string. > >> +@item @var{type} >> +If need create an autoload-breakpoint, this is the type in char >> +include H (hardware) and S (software). > > This is the type of the autoload-breakpoint to create, either > @samp{H} (for ``hardware'') or @samp{S} (for ``software''). > >> +@item @var{ignore_num} >> +If need create an autoload-breakpoint, this is the ignore_num in hex string. > > This is the ignore count of the autoload-breakpoint to create, > encoded as a hex string. > >> +@item @var{id}@samp{:}@samp{R} >> +@cindex @var{id}@samp{:}@samp{R} packet > > Please remove this @cindex entry, it is useless. > >> +@var{id} is the id in hex string format of this command want to remove. > > @var{id} is the number of the autoload-breakpoint that this command > wants to remove, encoded as a hex string. > >> +@item @var{id}@samp{:}@samp{C}@samp{:}@var{cmd_str} >> +@cindex @var{id}@samp{:}@samp{C}@samp{:}@var{cmd_str} packet > > No need for this @cindex. > >> +This packet add @var{cmd_str} to the commands list of >> +autoload-breakpoint @var{id}. > > This packet adds commands in @var{cmd_list} to the command list of > the autoload-breakpoint whose number is @var{id}. > >> If @var{cmd_str} is empty, >> +clear commands list of autoload-breakpoint @var{id}. > > If @var{cmd_str} is empty, the command list will be emptied. > >> +@var{cmd_str} is encoded to hex string. > ^^ > "as" > >> +@item @var{id}@samp{:}@samp{O}@samp{:}@var{condition_str} >> +@cindex @var{id}@samp{:}@samp{O}@samp{:}@var{condition_str} packet > > No need for this @cindex entry. > >> +This packet set @var{condition_str} as the condition of >> +autoload-breakpoint @var{id}. > > This packet sets the condition of the autoload-breakpoint @var{id} to > be as specified by @var{condition_str}. > >> If @var{condition_str} is empty, >> +clear condition of autoload-breakpoint @var{id}. > > If @var{condition_str} is empty, the autoload-breakpoint becomes > unconditional. > >> +@var{condition_str} is encoded to hex string. > ^^ > "as" > >> +@subsection Autoload-breakpints request packets >> +@table @samp >> +@item qBfP >> +@itemx qBsP >> +These packets request data in autoload-breakpoints base format >> +about autoload-breakpints from the stub. >> +@value{GDBN} sends @code{qBfP} to get the first piece >> +of data, and multiple @code{qTsP} to get additional pieces. >> +@end table >> + >> +@subsection Autoload-breakpints control packets >> +@table @samp >> +@item @samp{QBDP}@var{autoload-breakpoints base format} >> +@value{GDBN} and the stub use this packet to control >> +the autoload-breakpoints in the remote. The stub will translate >> +this packet through reportAsync Packets. >> +@end table > > No need to have a separate subsection for each packet. > > Also, I think you need to "connect the dots" by explaining the > connection between these packets and the format described above. It > is probably a good idea to start with the list of the packets and > their short description, and then follow up with the description of > their format. > > In sum, this documentation patch needs to be reworked. OK. I will do it according to your comments. Thanks, Hui