From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10533 invoked by alias); 25 Jun 2010 16:05:10 -0000 Received: (qmail 10516 invoked by uid 22791); 25 Jun 2010 16:05:08 -0000 X-SWARE-Spam-Status: No, hits=0.7 required=5.0 tests=AWL,BAYES_50,RCVD_IN_DNSWL_NONE,SPF_SOFTFAIL X-Spam-Check-By: sourceware.org Received: from mtaout22.012.net.il (HELO mtaout22.012.net.il) (80.179.55.172) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 25 Jun 2010 16:04:55 +0000 Received: from conversion-daemon.a-mtaout22.012.net.il by a-mtaout22.012.net.il (HyperSendmail v2007.08) id <0L4K00M00VAOQJ00@a-mtaout22.012.net.il> for gdb-patches@sources.redhat.com; Fri, 25 Jun 2010 19:04:52 +0300 (IDT) Received: from HOME-C4E4A596F7 ([77.127.155.52]) by a-mtaout22.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0L4K00F3NVC3ZHE0@a-mtaout22.012.net.il>; Fri, 25 Jun 2010 19:04:52 +0300 (IDT) Date: Fri, 25 Jun 2010 16:05:00 -0000 From: Eli Zaretskii Subject: Re: Better MI memory commands In-reply-to: <201006251232.55281.vladimir@codesourcery.com> To: Vladimir Prus Cc: gdb-patches@sources.redhat.com Reply-to: Eli Zaretskii Message-id: <837hlndrni.fsf@gnu.org> References: <201006251232.55281.vladimir@codesourcery.com> 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: 2010-06/txt/msg00580.txt.bz2 > From: Vladimir Prus > Date: Fri, 25 Jun 2010 12:32:55 +0400 > > The attached patch makes a few changes in MI memory commands, with the > goal of making it easy for frontend to just display memory view, even > around the boundaries of accessible memory. Thanks. I have a few comments about the documentation part of the patch. > +@item @var{count} > +The number of bytes to read. This should be an integer literal. ^^ Two spaces here, please. > +The commands attempts to read memory at the specified address. ^^^^^^^^^^^^ "This command" > + When a > +memory map is available A cross-reference here to where memory maps are described would be useful. > + regions marked as unreadable in the memory > +map will be skipped. In addition, when @value{GDBN} encouters an error ^^ Two spaces. > +reading a memory range, it will attempt to find readable subrange at ^^^^^^^^^^^^^^^^^^ "a readable subrange" > +the beginning and range. Essentially, this command will make ^^ ^^ Something ("end of the"?) is missing here. And two spaces between sentences. I'm not sure I completely understand this part: In addition, when @value{GDBN} encouters an error reading a memory range, it will attempt to find readable subrange [...] What if there are two or more non-contiguous sub-ranges at the beginning -- will GDB find and read both of them? More generally, what happens if the specified range has several disjoint portions that are not readable? I'm asking because your description seems to imply that we only look for the first readable address after start and the last readable address before the end of the specified region. IOW, it sounds like unreadable sub-regions in the middle of the region are not supported. If my understanding is correct, then the following sentence > + Essentially, this command will make > +reasonable effort to return all readable memory content within the > +requested range. is at least inaccurate, if not misleading ("all readable memory within the range"). > +The output of the command has a result record named @samp{memory}, ^^^^^^^^^^^^^^^^^^^ Perhaps "is a result record"? And what is the importance of naming this record `memory'? why is the name important here? > +@item contents > +The contents of the memory block, as hex-encoded string of bytes. But your example shows this: > + contents="01000000020000000300"@}] This doesn't look like a ``string of bytes'' to me. Or maybe I don't understand what you meant by that? > +@item @var{contents} > +The hex-encoded bytes to write. The size of this parameter determines > +how many bytes should be written.^^^^^^^^ You probably meant "the value", not "the size". > + Otherwise, if there's a readable subrange at the end, it will be > + completely and returned. Any readable subranges before it (obviously, ^^ "read" is missing here. > + The purpose of this function is to handle a read across a boundary of > + accessible memory in a case when memory map is not available. The above > + restrictions are fine for this case, but will give incorrect results if > + the memory is 'patchy'. However, supporting 'patchy' memory would require > + trying to read every single byte, and it seems unacceptable solution. > + Explicit memory map is recommended for this case -- and > + target_read_memory_robust will take care of reading multiple ranges then. At least some of this comment should IMO be in the manual. Thanks.