From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:42991)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1TC9G0-0005Jv-QT
	for qemu-devel@nongnu.org; Thu, 13 Sep 2012 09:15:07 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1TC9Fw-0007W9-Cd
	for qemu-devel@nongnu.org; Thu, 13 Sep 2012 09:15:00 -0400
Received: from mx1.redhat.com ([209.132.183.28]:14687)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1TC9Fw-0007W4-4G
	for qemu-devel@nongnu.org; Thu, 13 Sep 2012 09:14:56 -0400
From: Markus Armbruster <armbru@redhat.com>
References: <1347448378-23915-1-git-send-email-owasserm@redhat.com>
	<1347448378-23915-2-git-send-email-owasserm@redhat.com>
Date: Thu, 13 Sep 2012 15:14:48 +0200
In-Reply-To: <1347448378-23915-2-git-send-email-owasserm@redhat.com> (Orit
	Wasserman's message of "Wed, 12 Sep 2012 14:12:56 +0300")
Message-ID: <87ligef4jb.fsf@blackfin.pond.sub.org>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH v2 1/3] Refactor inet_connect_opts function
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Orit Wasserman <owasserm@redhat.com>
Cc: kwolf@redhat.com, aliguori@us.ibm.com, akong@redhat.com, mst@redhat.com, quintela@redhat.com, mdroth@linux.vnet.ibm.com, qemu-devel@nongnu.org, pbonzini@redhat.com, lcapitulino@redhat.com

One more...

Orit Wasserman <owasserm@redhat.com> writes:

[...]
> +static int inet_connect_addr(struct addrinfo *addr, bool block,
> +                             bool *in_progress, Error **errp)

Parameter errp is unused.

> +{
> +    char uaddr[INET6_ADDRSTRLEN + 1];
> +    char uport[33];
> +    int sock, rc;
> +
> +    if (getnameinfo((struct sockaddr *)addr->ai_addr, addr->ai_addrlen,
> +                    uaddr, INET6_ADDRSTRLEN, uport, 32,
> +                    NI_NUMERICHOST | NI_NUMERICSERV)) {
> +        fprintf(stderr, "%s: getnameinfo: oops\n", __func__);
> +        return -1;
> +    }
> +    sock = qemu_socket(addr->ai_family, addr->ai_socktype, addr->ai_protocol);
> +    if (sock < 0) {
> +        fprintf(stderr, "%s: socket(%s): %s\n", __func__,
> +                inet_strfamily(addr->ai_family), strerror(errno));
> +        return -1;
> +    }
> +    setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
> +    if (!block) {
> +        socket_set_nonblock(sock);
> +    }
> +    /* connect to peer */
> +    do {
> +        rc = 0;
> +        if (connect(sock, addr->ai_addr, addr->ai_addrlen) < 0) {
> +            rc = -socket_error();
>          }
> -        setsockopt(sock,SOL_SOCKET,SO_REUSEADDR,(void*)&on,sizeof(on));
> -        if (!block) {
> -            socket_set_nonblock(sock);
> +    } while (rc == -EINTR);
> +
> +    if (!block && QEMU_SOCKET_RC_INPROGRESS(rc)) {
> +        if (in_progress) {
> +            *in_progress = true;
>          }
> -        /* connect to peer */
> -        do {
> -            rc = 0;
> -            if (connect(sock, e->ai_addr, e->ai_addrlen) < 0) {
> -                rc = -socket_error();
> -            }
> -        } while (rc == -EINTR);
> -
> -  #ifdef _WIN32
> -        if (!block && (rc == -EINPROGRESS || rc == -EWOULDBLOCK
> -                       || rc == -WSAEALREADY)) {
> -  #else
> -        if (!block && (rc == -EINPROGRESS)) {
> -  #endif
> -            if (in_progress) {
> -                *in_progress = true;
> -            }
> -        } else if (rc < 0) {
> -            if (NULL == e->ai_next)
> -                fprintf(stderr, "%s: connect(%s,%s,%s,%s): %s\n", __FUNCTION__,
> -                        inet_strfamily(e->ai_family),
> -                        e->ai_canonname, uaddr, uport, strerror(errno));
> -            closesocket(sock);
> -            continue;
> +    } else if (rc < 0) {
> +        closesocket(sock);
> +        return -1;
> +    }
> +    return sock;
> +}
> +
> +int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp)
> +{
> +    struct addrinfo *res, *e;
> +    int sock = -1;
> +    bool block = qemu_opt_get_bool(opts, "block", 0);
> +
> +    res = inet_parse_connect_opts(opts, errp);
> +    if (!res) {
> +        return -1;
> +    }
> +
> +    if (in_progress) {
> +        *in_progress = false;
> +    }
> +
> +    for (e = res; e != NULL; e = e->ai_next) {
> +        sock = inet_connect_addr(e, block, in_progress, errp);
> +        if (in_progress && *in_progress) {
> +            return sock;
> +        } else if (sock >= 0) {
> +            break;
>          }
> -        freeaddrinfo(res);
> -        return sock;
>      }
> -    error_set(errp, QERR_SOCKET_CONNECT_FAILED);
> +    if (sock < 0) {
> +        error_set(errp, QERR_SOCKET_CONNECT_FAILED);

Necessary, because inet_connect_addr() doesn't do it.

Suggest to drop inet_connect_addr() parameter errp.

> +    }
>      freeaddrinfo(res);
> -    return -1;
> +    return sock;
>  }
>  
>  int inet_dgram_opts(QemuOpts *opts)