Re: [rfc] Correct semantics of target_read_partial, add target_read_whole

[Daniel Jacobowitz <drow@false.org>]

On Thu, Jun 22, 2006 at 08:24:54PM +0200, Mark Kettenis wrote:
> > Date: Wed, 21 Jun 2006 23:23:55 -0400
> > From: Daniel Jacobowitz <drow@false.org>
> > 
> > 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.
> 
> Ah, it all makes sense to me now.  I'm wondering whether we should
> "export" target_read_partial() (and target_write_partial()) at all.
> It's never right to use them except for implementing higher-level
> target read/write functions isn't it?

There's some messiness in kod.c.  But didn't we decide that was on the
chopping block to be removed?  Other than that, in my current working
tree there are absolutely no calls to these functions outside of
target.c.  So I think you're right: we can stop exporting them at all.

Vladimir made a related comment on gdb@ a week or two ago.  GDB has
developed too many ways to do the same thing.  We desperately need to
start bringing that number back down instead of up.  Since my patch
adds one (target_read_whole), I have an onus to remove at least one :-)
I will make the functions static in the next version of this patch.

And, thanks a lot for taking a look at these changes.  I can't say this
enough - I realize the work I'm doing is outside of the usual areas for
some of the other maintainers, but I rely on feedback to keep GDB the
best it can be!

> Agreed.  I have a (small) concern that the introduction of
> target_read_whole() will cause confusion with target_read().  Perhaps
> a better name would be target_read_alloc?

Sounds reasonable to me.  I couldn't think of a better name at the
time.

> You might consider commiting the sparc-tdep.c change seperately; it's
> "obvious".

Since I have to redo these patches anyway, and I've already put in a
couple of other conflicting bits, I might as well.  I'll do that later.

-- 
Daniel Jacobowitz
CodeSourcery