From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:58704) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RVLBf-0001CE-79 for qemu-devel@nongnu.org; Tue, 29 Nov 2011 05:45:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RVLBY-0005dS-G9 for qemu-devel@nongnu.org; Tue, 29 Nov 2011 05:45:19 -0500 Received: from mail-iy0-f173.google.com ([209.85.210.173]:45493) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RVLBY-0005dN-D5 for qemu-devel@nongnu.org; Tue, 29 Nov 2011 05:45:12 -0500 Received: by iakk32 with SMTP id k32so11828897iak.4 for ; Tue, 29 Nov 2011 02:45:11 -0800 (PST) Message-ID: <4ED53644.3060908@gmail.com> Date: Wed, 30 Nov 2011 04:45:08 +0900 From: Benjamin MIME-Version: 1.0 References: <1322225387-15073-1-git-send-email-mlspirat42@gmail.com> <4ED5168E.5080304@gmail.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] Support for UDP unicast network backend List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: andreas.faerber@web.de, jan.kiszka@web.de, qemu-devel@nongnu.org, Benjamin MARSILI On 11/29/11 18:47, Stefan Hajnoczi wrote: > On Tue, Nov 29, 2011 at 5:29 PM, Benjamin wrote: >> On 11/28/11 20:39, Stefan Hajnoczi wrote: >>> >>> On Fri, Nov 25, 2011 at 12:49 PM, Benjamin wrote: >>>> >>>> + fd = qemu_socket(PF_INET, SOCK_DGRAM, 0); >>>> + if (fd< 0) { >>>> + perror("socket(PF_INET, SOCK_DGRAM)"); >>>> + return -1; >>>> + } >>>> + val = 1; >>>> + ret = setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, >>>> + (const char *)&val, sizeof(val)); >>>> + if (ret< 0) { >>>> + perror("setsockopt(SOL_SOCKET, SO_REUSEADDR)"); >>> >>> >>> Please avoid leaking the file descriptor on error: >>> closesocket(fd); >>> >>> Since existing code also does this it may be more appropriate to send >>> a follow-up patch that cleans up all of net/socket.c. >>> >>> Reviewed-by: Stefan Hajnoczi >>> >>> Stefan >> >> >> I can do that. However is it really a leak considering the fact that >> the program will call exit just after? >> If it's a matter of consistency and coding style I would understand >> though. > > net/socket.c should not make assumptions about the main program > exiting after an error. NICs can be added at runtime using netdev_add > and that should not leak file descriptors. > Ah, indeed, I mixed up "err" and "perror" since I'm used to calling "warn" instead of perror. >> One more thing, git-format-patch added a "From" field to the header and >> caused this glitch in the mail. I thought git-send-mail or the mail >> server would handle it well but apparently not: >> >> From 2f5b85fcadcfee3b75a6a21dc86d10b945c99f0f Mon Sep 17 00:00:00 2001 >> From: Benjamin MARSILI >> >> git-am didn't complain with the patch that I sent but it may break after >> gmail relayed it >> (http://lists.gnu.org/archive/html/qemu-devel/2011-11/msg03152.html). >> The second from header is interpreted as text... Should I remove the >> first "From" field before sending the patch? > > This is normal and is not a problem. Your git commit is authored by > Benjamin MARSILI but you sent the mail from > Benjamin. > > git-am will apply the patch with Benjamin MARSILI > as the author and it will forget about Benjamin > . This is usually what you want - it let's you > credit commits to other people but send the patch emails on their > behalf. > > Stefan Great, thank you, sending v3 soon. Benjamin