From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:59739)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <owasserm@redhat.com>) id 1TCCWj-0000Et-7v
	for qemu-devel@nongnu.org; Thu, 13 Sep 2012 12:44:35 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <owasserm@redhat.com>) id 1TCCWh-00029A-Tb
	for qemu-devel@nongnu.org; Thu, 13 Sep 2012 12:44:29 -0400
Received: from mx1.redhat.com ([209.132.183.28]:54178)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <owasserm@redhat.com>) id 1TCCWh-00028j-90
	for qemu-devel@nongnu.org; Thu, 13 Sep 2012 12:44:27 -0400
Message-ID: <50520D72.6090300@redhat.com>
Date: Thu, 13 Sep 2012 19:44:34 +0300
From: Orit Wasserman <owasserm@redhat.com>
MIME-Version: 1.0
References: <1347448378-23915-1-git-send-email-owasserm@redhat.com>
	<1347448378-23915-2-git-send-email-owasserm@redhat.com>
	<87ligef4jb.fsf@blackfin.pond.sub.org>
In-Reply-To: <87ligef4jb.fsf@blackfin.pond.sub.org>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
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: Markus Armbruster <armbru@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

On 09/13/2012 04:14 PM, Markus Armbruster wrote:
> 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.
> 
no objections here
>> +    }
>>      freeaddrinfo(res);
>> -    return -1;
>> +    return sock;
>>  }
>>  
>>  int inet_dgram_opts(QemuOpts *opts)