qemu-nbd.c : fix memory leak

Signed-off-by: Li Zhi Hui <zhihuili@linux.vnet.ibm.com>
---
 qemu-nbd.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

On 12/02/2011 11:34 AM, Li Zhi Hui wrote:
> Signed-off-by: Li Zhi Hui<zhihuili@linux.vnet.ibm.com>
> ---
>   qemu-nbd.c |    4 +++-
>   1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 291cba2..ab7fa6c 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -500,8 +500,10 @@ int main(int argc, char **argv)
>           sharing_fds[0] = tcp_socket_incoming(bindto, port);
>       }
>
> -    if (sharing_fds[0] == -1)
> +    if (sharing_fds[0] == -1) {
> +        g_free(sharing_fds);
>           return 1;
> +    }
>
>       if (device) {
>           int ret;
Zhihui,

Kernel should free all memory used by the process after it exits.  So  
there's no memory leak even without explicit free.

Am 02.12.2011 08:28, schrieb Mark Wu:
> On 12/02/2011 11:34 AM, Li Zhi Hui wrote:
>> Signed-off-by: Li Zhi Hui<zhihuili@linux.vnet.ibm.com>
>> ---
>>   qemu-nbd.c |    4 +++-
>>   1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/qemu-nbd.c b/qemu-nbd.c
>> index 291cba2..ab7fa6c 100644
>> --- a/qemu-nbd.c
>> +++ b/qemu-nbd.c
>> @@ -500,8 +500,10 @@ int main(int argc, char **argv)
>>           sharing_fds[0] = tcp_socket_incoming(bindto, port);
>>       }
>>
>> -    if (sharing_fds[0] == -1)
>> +    if (sharing_fds[0] == -1) {
>> +        g_free(sharing_fds);
>>           return 1;
>> +    }
>>
>>       if (device) {
>>           int ret;
> Zhihui,
>
> Kernel should free all memory used by the process after it exits.  So  
> there's no memory leak even without explicit free.

That's correct. Nevertheless fixing this helps to find other more important
memory leaks with static or dynamic code analyzers like cppcheck or 
valgrind.

Reviewed-by: Stefan Weil <sw@weilnetz.de>

On 12/02/2011 08:42 AM, Stefan Weil wrote:
>>>
>>> -    if (sharing_fds[0] == -1)
>>> +    if (sharing_fds[0] == -1) {
>>> +        g_free(sharing_fds);
>>>           return 1;
>>> +    }
>>>
>>>       if (device) {
>>>           int ret;
>> Zhihui,
>>
>> Kernel should free all memory used by the process after it exits.  So
>> there's no memory leak even without explicit free.
>
> That's correct. Nevertheless fixing this helps to find other more important
> memory leaks with static or dynamic code analyzers like cppcheck or
> valgrind.
>
> Reviewed-by: Stefan Weil <sw@weilnetz.de>

Please hold off, I'm about to introduce large changes to qemu-nbd for 
asynchronous I/O (including getting rid of sharing_fds in favor of the 
main loop) and and I'd rather avoid gratuitous conflicts.

Paolo

Thank you very much!

On Fri, Dec 02, 2011 at 08:48:50AM +0100, Paolo Bonzini wrote:
> On 12/02/2011 08:42 AM, Stefan Weil wrote:
> >>>
> >>>-    if (sharing_fds[0] == -1)
> >>>+    if (sharing_fds[0] == -1) {
> >>>+        g_free(sharing_fds);
> >>>          return 1;
> >>>+    }
> >>>
> >>>      if (device) {
> >>>          int ret;
> >>Zhihui,
> >>
> >>Kernel should free all memory used by the process after it exits.  So
> >>there's no memory leak even without explicit free.
> >
> >That's correct. Nevertheless fixing this helps to find other more important
> >memory leaks with static or dynamic code analyzers like cppcheck or
> >valgrind.
> >
> >Reviewed-by: Stefan Weil <sw@weilnetz.de>
> 
> Please hold off, I'm about to introduce large changes to qemu-nbd
> for asynchronous I/O (including getting rid of sharing_fds in favor
> of the main loop) and and I'd rather avoid gratuitous conflicts.

Okay, let's wait for Paolo's changes.

Stefan