Re: [PATCH v19 08/20] io: add qio_channel_readv_full_all_eof & qio_channel_readv_full_all helpers

[Jag Raman]

> On Jan 14, 2021, at 11:27 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> 
> On Thu, Jan 14, 2021 at 10:40:03AM -0500, Jagannathan Raman wrote:
>> +int qio_channel_readv_full_all(QIOChannel *ioc,
>> +                               const struct iovec *iov,
>> +                               size_t niov,
>> +                               int **fds, size_t *nfds,
>> +                               Error **errp)
>> {
>> -    int ret = qio_channel_readv_all_eof(ioc, iov, niov, errp);
>> +    int ret = qio_channel_readv_full_all_eof(ioc, iov, niov, fds, nfds, 
>> errp);
>> 
>>     if (ret == 0) {
>> -        ret = -1;
>>         error_setg(errp,
>>                    "Unexpected end-of-file before all bytes were read");
> 
> qio_channel_readv_full_all_eof() can read file descriptors but no data
> and return 0.
> 
> Here that case is converted into an error and the file descriptors
> aren't closed, freed, and fds/nfds isn't cleared.

That’s a valid point. I’m wondering if the fix for this case should be in
qio_channel_readv_full_all_eof(), instead of here.

qio_channel_readv_full_all_eof() should probably return error (-1) if the
amount of data read does not match iov_size(). If the caller is only expecting
to read fds, and not any data, it would indicate that by setting iov to NULL
and/or setting niov=0. If the caller is setting these parameters, it means it is
expecting data.Does that sound good?

Thanks!