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

Vladimir Prus
2006-06-24T10:06:37Z

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