Re: [patch/rfc] target read/write partial

[Daniel Jacobowitz <drow at mvista dot com>]

On Wed, Oct 15, 2003 at 07:09:16PM -0400, Andrew Cagney wrote:
> >On Wed, Oct 15, 2003 at 06:16:28PM -0400, Andrew Cagney wrote:
> >
> >>Hello,
> >>
> >>This patch adds target read/write partial methods.
> >>
> >>It's almost ready for prime time.  I want to first see through some 
> >>other target cleanups namely:
> >>+  /* FIXME: cagney/2003-10-15: This code should walk down the target
> >>+     stack looking for a stratum that supports the mechanism.
> >>+     Unfortunatly, there isn't a per-target-stack chain to walk round.
> >>+     Catch-22.  */
> >>and a s/target_ops/target/ transformation.
> >
> >
> >Preferably not target - didn't someone suggest gdbtarg?  Or maybe
> >gdb_target.
> 
> ... you mean someone actually likes gdbtarg :-)

Not all that much.  But I really dislike "target".

> >>Note that it includes:
> >>+  /* Transfer up-to LEN bytes of memory starting at OFFSET.  */
> >>+  TARGET_OBJECT_MEMORY
> >>I'm going to need that when implementing a per-target 
> >>CONVERT_FROM_FUNC_PTR_ADDR.
> >
> >
> >How is that different from a memory read, which we've already got? I am
> >guessing that it's because you want to support partial memory
> >operations (avoid packet size limits), but you never explained your
> >goal.
> 
> I previously wrote:
> 
> >In the light of Joel's "upload/download" commands, and lessons (gdb/589) 
> >learn't from similar interfaces, and a realization that I need this for 
> >memory:
> ...
> >- it takes an explicit target vector
> >- there are both read and write variants (instead of query)
> >- it, what the heck, takes a LONGEST
> >- it makes the fact that the method isn't expected to perform a full 
> >transfer explicit
> 
> gdb/589 is yet another example of how the existing code always did 
> partial xfers, yet no one noticed.  Have a look at how many levels of 
> GDB code try to locally solve the partial transfer problem when 
> generic_load is called.
> 
> The only significant difference is the addition of an explicit target 
> vector.  But that's really significant.  I should probably comment out 
> TARGET_OBJECT_MEMORY until a target implements it.

Yes, please.  Anyway, I see where you're going now.

> >>+  /* Map pre-existing objects onto letters.  DO NOT do this for new
> >>+     objects!!!  Instead specify new query packets.  */
> >
> >
> >Could that be a little clearer - I had to read the code a couple of
> >times to figure out what you meant.  I guess you just want to say that
> >there's no need to use single letters?
> 
> The code abuses:
> 
> 	<letter> + <annex>
> 
> as a way of generating packets.  The entire qK* and qR* packet maps have 
> been kidnapped by this.  The un-approved RedBoot patches did the same 
> with qM*.
> 
> I'll expand the comment.

Do we need a standard terminator here too - colon or semicolon again?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer