This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [rfc] Correct semantics of target_read_partial, add target_read_whole
- From: Vladimir Prus <ghost at cs dot msu dot su>
- To: gdb-patches at sources dot redhat dot com
- Date: Sat, 24 Jun 2006 14:06:07 +0400
- Subject: Re: [rfc] Correct semantics of target_read_partial, add target_read_whole
- References: <20060622032355.GA27566@nevyn.them.org>
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