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.