Re: [Qemu-devel] [PATCH] qemu-char: reset errno before qemu char write or read action

[Paolo Bonzini <pbonzini@redhat.com>]

On 02/07/2018 02:49, xinhua.Cao wrote:
> In the tcp_chr_write function, we checked errno,
> but errno was not reset before a read or write operation.
> Therefore, this check of errno's actions is often
> incorrect after EAGAIN has occurred.
> We reset errno before reading and writing to
> ensure the correctness of errno's judgment

You should explain why this is a problem, because all the places you
modified are checking that the read or write has returned -1.  In that
case, errno must have been modified and it is unnecessary to write 0.

Thanks,

Paolo

> Signed-off-by: xinhua.Cao <caoxinhua@huawei.com>
> ---
>  chardev/char-fe.c | 1 +
>  chardev/char.c    | 2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/chardev/char-fe.c b/chardev/char-fe.c
> index b1f228e..d96ca6f 100644
> --- a/chardev/char-fe.c
> +++ b/chardev/char-fe.c
> @@ -69,6 +69,7 @@ int qemu_chr_fe_read_all(CharBackend *be, uint8_t *buf, int len)
>  
>      while (offset < len) {
>      retry:
> +        errno = 0;
>          res = CHARDEV_GET_CLASS(s)->chr_sync_read(s, buf + offset,
>                                                    len - offset);
>          if (res == -1 && errno == EAGAIN) {
> diff --git a/chardev/char.c b/chardev/char.c
> index 76d866e..3387442 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -85,6 +85,7 @@ static void qemu_chr_write_log(Chardev *s, const uint8_t *buf, size_t len)
>  
>      while (done < len) {
>      retry:
> +        errno = 0;
>          ret = write(s->logfd, buf + done, len - done);
>          if (ret == -1 && errno == EAGAIN) {
>              g_usleep(100);
> @@ -109,6 +110,7 @@ static int qemu_chr_write_buffer(Chardev *s,
>      qemu_mutex_lock(&s->chr_write_lock);
>      while (*offset < len) {
>      retry:
> +        errno = 0;
>          res = cc->chr_write(s, buf + *offset, len - *offset);
>          if (res < 0 && errno == EAGAIN && write_all) {
>              g_usleep(100);
>