Re: [Qemu-devel] [PATCH] qemu-char: delete send_all/recv_all helper methods

[Paolo Bonzini <pbonzini@redhat.com>]

On 23/12/2015 14:59, Daniel P. Berrange wrote:
> The qemu-char.c contains two helper methods send_all
> and recv_all. These are in fact declared in sockets.h
> so ought to have been in util/qemu-sockets.c. For added
> fun the impl of recv_all is completely missing on Win32.
> 
> Fortunately there is only a single caller of these
> methods, the TPM passthrough code, which is only
> ever compiled on Linux. With only a single caller
> these helpers are not compelling enough to keep so
> inline them in the TPM code, avoiding the need to
> fix the missing recv_all on Win32.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  hw/tpm/tpm_passthrough.c | 29 ++++++++++++++++++--
>  include/qemu/sockets.h   |  2 --
>  qemu-char.c              | 71 ------------------------------------------------
>  3 files changed, 27 insertions(+), 75 deletions(-)
> 
> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> index be160c1..ab526db 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -83,12 +83,37 @@ static void tpm_passthrough_cancel_cmd(TPMBackend *tb);
>  
>  static int tpm_passthrough_unix_write(int fd, const uint8_t *buf, uint32_t len)
>  {
> -    return send_all(fd, buf, len);
> +    int ret, remain;
> +
> +    remain = len;
> +    while (len > 0) {
> +        ret = write(fd, buf, remain);
> +        if (ret < 0) {
> +            if (errno != EINTR && errno != EAGAIN) {
> +                return -1;
> +            }
> +        } else if (ret == 0) {
> +            break;
> +        } else {
> +            buf += ret;
> +            remain -= ret;
> +        }
> +    }
> +    return len - remain;
>  }
>  
>  static int tpm_passthrough_unix_read(int fd, uint8_t *buf, uint32_t len)
>  {
> -    return recv_all(fd, buf, len, true);
> +    int ret;
> + reread:
> +    ret = read(fd, buf, len);
> +    if (ret < 0) {
> +        if (errno != EINTR && errno != EAGAIN) {
> +            return -1;
> +        }
> +        goto reread;
> +    }
> +    return ret;
>  }
>  
>  static uint32_t tpm_passthrough_get_size_from_buffer(const uint8_t *buf)
> diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
> index 74c692d..0ff3a72 100644
> --- a/include/qemu/sockets.h
> +++ b/include/qemu/sockets.h
> @@ -40,8 +40,6 @@ int socket_set_nodelay(int fd);
>  void qemu_set_block(int fd);
>  void qemu_set_nonblock(int fd);
>  int socket_set_fast_reuse(int fd);
> -int send_all(int fd, const void *buf, int len1);
> -int recv_all(int fd, void *buf, int len1, bool single_read);
>  
>  #ifdef WIN32
>  /* Windows has different names for the same constants with the same values */
> diff --git a/qemu-char.c b/qemu-char.c
> index 66703e3..720235d 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -695,77 +695,6 @@ static CharDriverState *qemu_chr_open_mux(const char *id,
>  }
>  
>  
> -#ifdef _WIN32
> -int send_all(int fd, const void *buf, int len1)
> -{
> -    int ret, len;
> -
> -    len = len1;
> -    while (len > 0) {
> -        ret = send(fd, buf, len, 0);
> -        if (ret < 0) {
> -            errno = WSAGetLastError();
> -            if (errno != WSAEWOULDBLOCK) {
> -                return -1;
> -            }
> -        } else if (ret == 0) {
> -            break;
> -        } else {
> -            buf += ret;
> -            len -= ret;
> -        }
> -    }
> -    return len1 - len;
> -}
> -
> -#else
> -
> -int send_all(int fd, const void *_buf, int len1)
> -{
> -    int ret, len;
> -    const uint8_t *buf = _buf;
> -
> -    len = len1;
> -    while (len > 0) {
> -        ret = write(fd, buf, len);
> -        if (ret < 0) {
> -            if (errno != EINTR && errno != EAGAIN)
> -                return -1;
> -        } else if (ret == 0) {
> -            break;
> -        } else {
> -            buf += ret;
> -            len -= ret;
> -        }
> -    }
> -    return len1 - len;
> -}
> -
> -int recv_all(int fd, void *_buf, int len1, bool single_read)
> -{
> -    int ret, len;
> -    uint8_t *buf = _buf;
> -
> -    len = len1;
> -    while ((len > 0) && (ret = read(fd, buf, len)) != 0) {
> -        if (ret < 0) {
> -            if (errno != EINTR && errno != EAGAIN) {
> -                return -1;
> -            }
> -            continue;
> -        } else {
> -            if (single_read) {
> -                return ret;
> -            }
> -            buf += ret;
> -            len -= ret;
> -        }
> -    }
> -    return len1 - len;
> -}
> -
> -#endif /* !_WIN32 */
> -
>  typedef struct IOWatchPoll
>  {
>      GSource parent;
> 

Queued, thanks.

Paolo