From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13780 invoked by alias); 29 Jun 2006 09:34:08 -0000 Received: (qmail 13771 invoked by uid 22791); 29 Jun 2006 09:34:07 -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; Thu, 29 Jun 2006 09:34:01 +0000 Received: from list by ciao.gmane.org with local (Exim 4.43) id 1FvsuI-0003PQ-AF for gdb-patches@sources.redhat.com; Thu, 29 Jun 2006 11:33:54 +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 ; Thu, 29 Jun 2006 11:33:54 +0200 Received: from ghost by zigzag.lvk.cs.msu.su with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Thu, 29 Jun 2006 11:33:54 +0200 To: gdb-patches@sources.redhat.com From: Vladimir Prus Subject: Re: [PATCH] Cleanup target memory reading Date: Thu, 29 Jun 2006 09:34:00 -0000 Message-ID: References: <200606281155.59166.ghost@cs.msu.su> <200606282130.k5SLUbwK011892@elgar.sibelius.xs4all.nl> <20060629024229.GB32465@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/msg00399.txt.bz2 Daniel Jacobowitz wrote: >> Shouldn't we "fix" target_read() such that it uses >> target_read_partial() instead? > It already does. It's just not an operation that is often useful. > > We've got four, if you include my pending (pending rewrite, that is) > patch: > > - target_read_partial. This is now an implementation detail. It > reads some bytes. Who knows how many. > - target_read. It reads a fixed, requested number of bytes. > - xfer_using_stratum. Similar, but allow the request to be split and > parts handled by different strata. This is always the right choice for > memory, at least for consistency. > - target_read_object_alloc, which is my current thinking of a name > for what was target_read_whole in my patch. This is the basically > always appropriate choice to fetch an "object" other than memory, since > we don't have any way to find out what size it is otherwise, so we > can't request a particular size. > > I don't want that last one to use the second one, because it's prone to > doing this: > > - target_read_object_alloc > - target_read 512 bytes > - target_read_partial gets 500 > - Second target_read_partial for 12 bytes > - Another target_read > > And we'd much rather group them in larger batches than twelve byte > reads. > > I think Vlad's basically garbage collecting target_read, since #3 and > #4 from my list cover all the needs we've got right now. Does this > seem reasonable? I think we've got too many ways to do this, and could > do with fewer. Yea, garbage collecting is precisely what I do here. The target_read is not very useful to reading memory, since it's ignores strata, and it's not very convenient for reading objects, since your upcoming target_read_object_alloc does not require a caller to allocate buffer of unknown size in advance. If fact, I wanted to kill target_write as well, but won't do this until either you and I implement qXfer write operation. > I'd even suggest a pass over the other memory reading functions, > pruning. For instance, the ones that take a frame. We know why they > were added, and it's in theory a good plan to associate memory reads > with frames, but it's not going to be completed any time in the > foreseeable future. We've got lots of what the GCC folks called an > incomplete transition when they were ranting about internal > cleanliness. I can do a second look, it this seems reasonable. - Volodya