Re: supporting all kinds of partially-<unavailable> enum target_object types

[Pedro Alves <palves at redhat dot com>]

On 11/07/2013 01:03 AM, Yao Qi wrote:
> On 10/25/2013 12:15 AM, Pedro Alves wrote:
>> We could do that by changing target_xfer_partial's
>> interface from:
>>
>>      LONGEST (*to_xfer_partial) (struct target_ops *ops,
>> 				enum target_object object, const char *annex,
>> 				gdb_byte *readbuf, const gdb_byte *writebuf,
>> 				ULONGEST offset, LONGEST len);
>>
>> to:
>>
>>      int (*to_xfer_partial) (struct target_ops *ops,
>> 				enum target_object object, const char *annex,
>> 				gdb_byte *readbuf, const gdb_byte *writebuf,
>> 				ULONGEST offset, ULONGEST len, ULONGEST *read_count);
>>
>> Where the target implementation would write how many bytes were read
>> to *READ_COUNT, instead of returning it. (The int return would then be just
>> be success/error, instead of the read length.)
> 
> How about "XFERED_COUT" or "XFERED_LEN"? since it may be used by write.

Yeah.  I guess XFERED_LEN to go with LEN.

> 
> Current to_xfer_partial returns one of three states, read length (> 0), 
> done (0), and error (< 0).  to_xfer_partial could return an enum to 
> represent these status, e.g.,
> 
> enum target_xfer_status
> {
>    TARGET_XFER_OK = 1,
>    TARGET_XFER_DONE = 0,
> 
>    TARGET_XFER_ERROR_IO = -1,
>    TARGET_XFER_ERROR_UNAVAILABLE = -2,
> };
> 
> When TARGET_XFER_OK is returned, *XFERED_COUNT is the number of bytes 
> read from target.  When TARGET_XFER_DONE is returned, to_xfer_partial 
> can't handle this request.  When TARGET_XFER_ERROR_XXX are returned, 
> *XFERED_COUNT is set properly.
> 
> What do you think about the interface like this?

Yes, something like that was what I was going for, when I added
enum target_xfer_error.  I think you've chosen 0 for
TARGET_XFER_DONE to try to avoid changing current code
unnecessarily.  IIRC, I mentioned back then that I didn't
encode an enum for success because different places treat "0"
differently.  If possible, cleaning that all up would be nice,
though I'm not sure we'd really be able to distinguish
TARGET_XFER_ERROR_IO from TARGET_XFER_DONE.

> When TARGET_XFER_DONE is returned, to_xfer_partial can't handle
> this request.

I think we need to pick a word other than "done" here.  "done"
sounds ambiguous with "ok", hence confusing, and it's actually
indicative that no transfer was done/possible.  Perhaps
TARGET_XFER_ERROR_NOT_SUPPORTED.

> 
>>
>> When returning TARGET_XFER_E_UNAVAILABLE, we'd return in
>> *READ_COUNT the number of unavailable bytes "read", or IOW, where
>> the next non-<unavailable> byte is, or LEN, if that comes before.
>>
>> So the (**1) case above would return TARGET_XFER_E_UNAVAILABLE
>> with *READ_COUNT set to 50.
>>
>> target_read_memory would be likewise adjusted, or a variant added.
>> And then, in specific case of values, read_value_memory would not have that
>> available_memory vector or any traceframe_number check, but instead
>> would call target_read_memory in a loop, and if TARGET_XFER_E_UNAVAILABLE
>> comes out, it'd mark READ_COUNT value bytes unavailable starting at
>> the requested address, and then continue reading from the previous
>> addr + READ_COUNT, rinse repeat, until the whole value was read in.
>>
>> This naturally implies pushing down the decision of whether
>> to return TARGET_XFER_E_UNAVAILABLE or something else
>> down to the target.  (Which kinds of leads back to tfile
>> itself reading from RO memory from file (though we could
> 
> IIUC, each target (tfile and remote, for example) should fetch 
> traceframe_info in its to_xfer_partial implementation, to decide whether 
> to return TARGET_XFER_E_UNAVAILABLE, right?

Yeah, for things that are in the traceframe info.  For remote,
we might end up extending qXfer similarly, to consult the target
directly avoiding having to list all kinds of objects in
traceframe info.

Thanks,
-- 
Pedro Alves