Re: [PULL v4 14/27] io: add qio_channel_readv_full_all_eof & qio_channel_readv_full_all helpers

[Max Reitz]

On 10.02.21 10:26, Stefan Hajnoczi wrote:
> From: Elena Ufimtseva <elena.ufimtseva@oracle.com>
>
> Adds qio_channel_readv_full_all_eof() and qio_channel_readv_full_all()
> to read both data and FDs. Refactors existing code to use these helpers.
>
> Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
> Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
> Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
> Acked-by: Daniel P. Berrangé <berrange@redhat.com>
> Message-id: 
> b059c4cc0fb741e794d644c144cc21372cad877d.1611938319.git.jag.raman@oracle.com
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   include/io/channel.h |  53 +++++++++++++++++++++++
>   io/channel.c         | 101 ++++++++++++++++++++++++++++++++++---------
>   2 files changed, 134 insertions(+), 20 deletions(-)

[...]

> diff --git a/io/channel.c b/io/channel.c
> index 0d4b8b5160..4555021b62 100644
> --- a/io/channel.c
> +++ b/io/channel.c

[...]

> @@ -135,20 +193,23 @@ int qio_channel_readv_all_eof(QIOChannel *ioc,
>       return ret;
>   }
>   
> -int qio_channel_readv_all(QIOChannel *ioc,
> -                          const struct iovec *iov,
> -                          size_t niov,
> -                          Error **errp)
> +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");
> -    } else if (ret == 1) {
> -        ret = 0;
> +        error_prepend(errp,
> +                      "Unexpected end-of-file before all data were read.");
> +        return -1;

This change breaks iotest 083 (i.e., it segfaults), because 
qio_channel_readv_full_all_eof() doesn’t set *errp when it returns 0, so 
there is no error to prepend.

Also, I think usually we don’t let error messages end in full stops.

Max