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
signature.asc
Description: OpenPGP digital signature
