From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15006 invoked by alias); 24 Jun 2006 10:06:37 -0000 Received: (qmail 14998 invoked by uid 22791); 24 Jun 2006 10:06:36 -0000 X-Spam-Check-By: sourceware.org Received: from main.gmane.org (HELO ciao.gmane.org) (80.91.229.2) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 24 Jun 2006 10:06:32 +0000 Received: from list by ciao.gmane.org with local (Exim 4.43) id 1Fu51z-000144-W8 for gdb-patches@sources.redhat.com; Sat, 24 Jun 2006 12:06:24 +0200 Received: from zigzag.lvk.cs.msu.su ([158.250.17.23]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Sat, 24 Jun 2006 12:06:23 +0200 Received: from ghost by zigzag.lvk.cs.msu.su with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Sat, 24 Jun 2006 12:06:23 +0200 To: gdb-patches@sources.redhat.com From: Vladimir Prus Subject: Re: [rfc] Correct semantics of target_read_partial, add target_read_whole Date: Sat, 24 Jun 2006 10:06:00 -0000 Message-ID: References: <20060622032355.GA27566@nevyn.them.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7Bit User-Agent: KNode/0.8.2 X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-06/txt/msg00372.txt.bz2 Daniel Jacobowitz wrote: > Originally, target_read_partial was supposed to read "however much it > could manage to" and then higher level functions were supposed to handle > the rest. But every existing implementation always reads enough data in > its first call; the one remote protocol implementation did so by issuing > as many packets as necessary, which defeated the point of the original > design. > > This patch adjusts the remote protocol layer not to do that. It also > promotes a useful function from auxv.c to target.c: > > +/* Wrappers to perform a full read of unknown size. OBJECT/ANNEX will > + be read using OPS. The return value will be -1 if the transfer > + fails or is not supported; 0 if the object is empty; and the length > + of the object otherwise. If a positive value is returned, a > + sufficiently large buffer will be allocated using xmalloc and > + returned in *BUF_P containing the contents of the object. > + > + This method should be used for objects sufficiently small to store > + in a single xmalloced buffer, when no fixed bound on the object's > + size is known in advance. Don't try to read TARGET_OBJECT_MEMORY > + through this function. */ > + > +extern LONGEST target_read_whole (struct target_ops *ops, > + enum target_object object, > + const char *annex, gdb_byte **buf_p); Dan, did you notice my post on gdb-devel about having two different methods of reading method. The post subject was: "target memory read/write methods". To quote: One way to read a memory is: - target_read_memory, which calls - xfer_using_stratum, which calls - target_xfer_partial (iterating over 'stratums'), which - goes to function pointer in target_ops This is the predominant method. Another way to read memory is: - get_target_memory{unsigned}, which calls: - target_read, which calls: - target_xfer_partial Your patch add target_read_whole, that calls 'target_read', which until now is used only by get_target_memory{unsigned}, which is used in 3 places in entire gdb. In addition 'xfer_using_stratum' already has some code to repeatedly call target_xfer_partial. I also might not understand what stratums are but should not they be used in all cases? Otherwise, behaviour of target_read_while for 'object = TARGET_OBJECT_MEMORY' will be different from the behaviour of 'target_read_memory'. I think that either: 1. There are way too many code paths for reading 2. There are way too few comments. Is seems to me that the right solution would be to have target_read_whole directly call xfer_using_stratum. Also, how you target_read_whole work when target_xfer_partial_p () return false? The 'target_read' function does not check it and 'target_xfer_partial' will just assert. Should 'target_read_whole' check for target_xfer_partial_p and fail is its false? - Volodya