Re: [Qemu-devel] [PATCH v2] Support for UDP unicast network backend

[Benjamin <mlspirat42@gmail.com>]

On 11/29/11 18:47, Stefan Hajnoczi wrote:
> On Tue, Nov 29, 2011 at 5:29 PM, Benjamin<mlspirat42@gmail.com>  wrote:
>> On 11/28/11 20:39, Stefan Hajnoczi wrote:
>>>
>>> On Fri, Nov 25, 2011 at 12:49 PM, Benjamin<mlspirat42@gmail.com>    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<stefanha@linux.vnet.ibm.com>
>>>
>>> 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<marsil_b@epitech.eu>
>>
>> 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<marsil_b@epitech.eu>  but you sent the mail from
> Benjamin<mlspirat42@gmail.com>.
>
> git-am will apply the patch with Benjamin MARSILI
> <marsil_b@epitech.eu>  as the author and it will forget about Benjamin
> <mlspirat42@gmail.com>.  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