Re: [Qemu-devel] [PATCH v3 1/5] util: add socket_set_fast_reuse function which will replace setting SO_REUSEADDR

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1711 bytes --]

On 09/10/2013 10:23 AM, Sebastian Ottlik wrote:

>>> +    if (ret < 0) {
>>> +        perror("setsockopt(SOL_SOCKET, SO_REUSEADDR)");
>>> +    }
>> This would be the first use of perror in this file; I'm not sure if that
>> is the right function, or if there is a better thing to be using (in
>> fact, returning -1 and letting the client decide whether to issue a
>> warning may even be better).
>>
> When I started writing the patch I was going to return the error and lat
> the client handle the issue. But the code in net/socket.c then becomes:
> 
>     ret = socket_set_fast_reuse(fd);
>     if (ret < 0) {
>         perror("setsockopt(SOL_SOCKET, SO_REUSEADDR)");
>         closesocket(fd);
>         return -1;
>     }
> 
> Which looked unclean to me, as the code implies assumptions about the
> implementation of socket_set_fast_reuse. One could also call
> perror("socket_set_fast_reuse()") but this would break the convention in
> the surrounding code of passing for the function that failed to perror.

Maybe a compromise?  Add a 'bool silent' flag to socket_set_fast_reuse,
and only issue perror() if the flag is false.  Existing callers that
don't care about failure (if we get fast reuse, great; if not, no huge
loss) pass false, existing callers that did their own error reporting
pass true to take advantage of the perror() on failure, and then you
aren't changing semantics at call sites.

But I'm just making this observation from the side; you might want to
get an opinion from an actual maintainer of this area of code on which
approach is best.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]