Re: [Qemu-devel] [PATCH 1/1] Coverity: Fix failure path for qemu_accept in migration

[Peter Maydell <peter.maydell@linaro.org>]

On 19 March 2014 11:13, Dr. David Alan Gilbert (git)
<dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Coverity defects 1005733 & 1005734 complain about passing a -ve value
> to closesocket in the error paths on incoming migration.
>
> Stash the error value and print it in the message (previously we gave
> no indication of the reason for the failure)
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  migration-tcp.c  | 11 ++++++-----
>  migration-unix.c | 11 ++++++-----
>  2 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/migration-tcp.c b/migration-tcp.c
> index 782572d..5c96cd3 100644
> --- a/migration-tcp.c
> +++ b/migration-tcp.c
> @@ -56,19 +56,20 @@ static void tcp_accept_incoming_migration(void *opaque)
>      socklen_t addrlen = sizeof(addr);
>      int s = (intptr_t)opaque;
>      QEMUFile *f;
> -    int c;
> +    int c, err;
>
>      do {
>          c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
> -    } while (c == -1 && socket_error() == EINTR);
> +        err = socket_error();
> +    } while (c == -1 && err == EINTR);
>      qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
>      closesocket(s);
>
>      DPRINTF("accepted migration\n");
>
> -    if (c == -1) {
> -        fprintf(stderr, "could not accept migration connection\n");
> -        goto out;
> +    if (c < 0) {

Why change the condition? Or alternatively, why use <0 here
but retain == -1 in the while condition above?

> +        fprintf(stderr, "could not accept migration connection (%d)\n", err);

Bit unfriendly not to convert the errno to a string.

thanks
-- PMM