Re: [Qemu-trivial] [Qemu-devel] [RESEND PATCH 1/3] socket: remove redundant check

[Markus Armbruster <armbru@redhat.com>]

Stefan Hajnoczi <stefanha@gmail.com> writes:

> On Wed, Aug 01, 2012 at 04:59:01PM +0800, Amos Kong wrote:
>> It's aleady in the end of loop, error should be set.
>> 
>> Signed-off-by: Amos Kong <akong@redhat.com>
>> ---
>>  qemu-sockets.c |    4 +---
>>  1 files changed, 1 insertions(+), 3 deletions(-)
>> 
>> diff --git a/qemu-sockets.c b/qemu-sockets.c
>> index 668fa93..c636882 100644
>> --- a/qemu-sockets.c
>> +++ b/qemu-sockets.c
>> @@ -181,9 +181,7 @@ int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp)
>>                  fprintf(stderr,"%s: bind(%s,%s,%d): %s\n", __FUNCTION__,
>>                          inet_strfamily(e->ai_family), uaddr, inet_getport(e),
>>                          strerror(errno));
>> -                if (!e->ai_next) {
>> -                    error_set(errp, QERR_SOCKET_BIND_FAILED);
>> -                }
>> +                error_set(errp, QERR_SOCKET_BIND_FAILED);
>>              }
>>          }
>>          closesocket(slisten);
>
> This isn't obvious.  It looks like the intent of the if (!e->ai_next) is
> to suppress the error so that the next iteration of the *outer* loop can
> succeed.
>
> Why is it okay to set QERR_SOCKET_BIND_FAILED?  We may have more
> addrinfos left to try in the outer loop.  They may succeed so we don't
> want an error in that case.

You are correct, and the patch is wrong.

See also related
http://lists.nongnu.org/archive/html/qemu-devel/2012-02/msg00772.html