From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52118) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VJQyG-0003gl-1y for qemu-devel@nongnu.org; Tue, 10 Sep 2013 12:39:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VJQy8-0007mE-N4 for qemu-devel@nongnu.org; Tue, 10 Sep 2013 12:39:19 -0400 Received: from ex-e-1.perimeter.fzi.de ([141.21.8.250]:23306) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VJQy8-0007mA-D3 for qemu-devel@nongnu.org; Tue, 10 Sep 2013 12:39:12 -0400 Message-ID: <522F4B2E.3080308@fzi.de> Date: Tue, 10 Sep 2013 18:39:10 +0200 From: Sebastian Ottlik MIME-Version: 1.0 References: <1378819619-20579-1-git-send-email-ottlik@fzi.de> <1378819619-20579-2-git-send-email-ottlik@fzi.de> <522F412F.9030205@redhat.com> <522F4775.1070606@fzi.de> <522F4A26.8080805@redhat.com> In-Reply-To: <522F4A26.8080805@redhat.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 1/5] util: add socket_set_fast_reuse function which will replace setting SO_REUSEADDR List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Jan Kiszka , Anthony Liguori , qemu-devel@nongnu.org, Stefan Hajnoczi On 10.09.2013 18:34, Eric Blake wrote: > 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. > This is probably the least intrusive approach, which is probably best without further maintainer input. I will wait and see if someone responds.