qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 2/2] plugins: Fix two resource leaks in connect_socket()
@ 2020-10-28 13:45 AlexChen
  2020-11-05  6:54 ` AlexChen
  2020-11-16 16:50 ` Thomas Huth
  0 siblings, 2 replies; 6+ messages in thread
From: AlexChen @ 2020-10-28 13:45 UTC (permalink / raw)
  To: alex.bennee, Laurent Vivier, mjt
  Cc: qemu-trivial, zhengchuan, qemu-devel, zhang.zhanghailiang

Either accept() fails or exits normally, we need to close the fd.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: AlexChen <alex.chen@huawei.com>
---
 contrib/plugins/lockstep.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c
index 319bd44b83..5aad50869d 100644
--- a/contrib/plugins/lockstep.c
+++ b/contrib/plugins/lockstep.c
@@ -268,11 +268,13 @@ static bool setup_socket(const char *path)
     socket_fd = accept(fd, NULL, NULL);
     if (socket_fd < 0 && errno != EINTR) {
         perror("accept socket");
+        close(fd);
         return false;
     }

     qemu_plugin_outs("setup_socket::ready\n");

+    close(fd);
     return true;
 }

-- 
2.19.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] plugins: Fix two resource leaks in connect_socket()
  2020-10-28 13:45 [PATCH 2/2] plugins: Fix two resource leaks in connect_socket() AlexChen
@ 2020-11-05  6:54 ` AlexChen
  2020-11-16 16:50 ` Thomas Huth
  1 sibling, 0 replies; 6+ messages in thread
From: AlexChen @ 2020-11-05  6:54 UTC (permalink / raw)
  To: alex.bennee, Laurent Vivier, mjt
  Cc: qemu-trivial, zhengchuan, qemu-devel, zhang.zhanghailiang

Kindly ping.

On 2020/10/28 21:45, AlexChen wrote:
> Either accept() fails or exits normally, we need to close the fd.
> 
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: AlexChen <alex.chen@huawei.com>
> ---
>  contrib/plugins/lockstep.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c
> index 319bd44b83..5aad50869d 100644
> --- a/contrib/plugins/lockstep.c
> +++ b/contrib/plugins/lockstep.c
> @@ -268,11 +268,13 @@ static bool setup_socket(const char *path)
>      socket_fd = accept(fd, NULL, NULL);
>      if (socket_fd < 0 && errno != EINTR) {
>          perror("accept socket");
> +        close(fd);
>          return false;
>      }
> 
>      qemu_plugin_outs("setup_socket::ready\n");
> 
> +    close(fd);
>      return true;
>  }
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] plugins: Fix two resource leaks in connect_socket()
  2020-10-28 13:45 [PATCH 2/2] plugins: Fix two resource leaks in connect_socket() AlexChen
  2020-11-05  6:54 ` AlexChen
@ 2020-11-16 16:50 ` Thomas Huth
  2020-11-17  1:13   ` Alex Chen
  1 sibling, 1 reply; 6+ messages in thread
From: Thomas Huth @ 2020-11-16 16:50 UTC (permalink / raw)
  To: AlexChen, alex.bennee, Laurent Vivier, mjt
  Cc: qemu-trivial, zhengchuan, qemu-devel, zhang.zhanghailiang

On 28/10/2020 14.45, AlexChen wrote:
> Either accept() fails or exits normally, we need to close the fd.
> 
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: AlexChen <alex.chen@huawei.com>
> ---
>  contrib/plugins/lockstep.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c
> index 319bd44b83..5aad50869d 100644
> --- a/contrib/plugins/lockstep.c
> +++ b/contrib/plugins/lockstep.c
> @@ -268,11 +268,13 @@ static bool setup_socket(const char *path)
>      socket_fd = accept(fd, NULL, NULL);

I think you could also simply close(fd) here instead, then you don't have to
do it twice below.

 Thomas


>      if (socket_fd < 0 && errno != EINTR) {
>          perror("accept socket");
> +        close(fd);
>          return false;
>      }
> 
>      qemu_plugin_outs("setup_socket::ready\n");
> 
> +    close(fd);
>      return true;
>  }
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] plugins: Fix two resource leaks in connect_socket()
  2020-11-16 16:50 ` Thomas Huth
@ 2020-11-17  1:13   ` Alex Chen
  2020-11-17 11:35     ` Alex Bennée
  0 siblings, 1 reply; 6+ messages in thread
From: Alex Chen @ 2020-11-17  1:13 UTC (permalink / raw)
  To: Thomas Huth, Alex Bennée
  Cc: zhang.zhanghailiang, qemu-trivial, mjt, Laurent Vivier,
	qemu-devel, zhengchuan, alex.bennee

On 2020/11/17 0:50, Thomas Huth wrote:
> On 28/10/2020 14.45, AlexChen wrote:
>> Either accept() fails or exits normally, we need to close the fd.
>>
>> Reported-by: Euler Robot <euler.robot@huawei.com>
>> Signed-off-by: AlexChen <alex.chen@huawei.com>
>> ---
>>  contrib/plugins/lockstep.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c
>> index 319bd44b83..5aad50869d 100644
>> --- a/contrib/plugins/lockstep.c
>> +++ b/contrib/plugins/lockstep.c
>> @@ -268,11 +268,13 @@ static bool setup_socket(const char *path)
>>      socket_fd = accept(fd, NULL, NULL);
> 
> I think you could also simply close(fd) here instead, then you don't have to
> do it twice below.
> 

Hi Thomas and Alex,
Thanks for your suggestion. It's a simple and effective solution.
Considering that the patch v3 has been queued by Alex Bennée,
May I modify this patch and then send patch v4?

Thanks,
Alex

> 
>>      if (socket_fd < 0 && errno != EINTR) {
>>          perror("accept socket");
>> +        close(fd);
>>          return false;
>>      }
>>
>>      qemu_plugin_outs("setup_socket::ready\n");
>>
>> +    close(fd);
>>      return true;
>>  }
>>
> 
> .
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] plugins: Fix two resource leaks in connect_socket()
  2020-11-17  1:13   ` Alex Chen
@ 2020-11-17 11:35     ` Alex Bennée
  2020-11-17 11:36       ` Thomas Huth
  0 siblings, 1 reply; 6+ messages in thread
From: Alex Bennée @ 2020-11-17 11:35 UTC (permalink / raw)
  To: Alex Chen
  Cc: Thomas Huth, zhang.zhanghailiang, qemu-trivial, mjt,
	Laurent Vivier, qemu-devel, zhengchuan


Alex Chen <alex.chen@huawei.com> writes:

> On 2020/11/17 0:50, Thomas Huth wrote:
>> On 28/10/2020 14.45, AlexChen wrote:
>>> Either accept() fails or exits normally, we need to close the fd.
>>>
>>> Reported-by: Euler Robot <euler.robot@huawei.com>
>>> Signed-off-by: AlexChen <alex.chen@huawei.com>
>>> ---
>>>  contrib/plugins/lockstep.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c
>>> index 319bd44b83..5aad50869d 100644
>>> --- a/contrib/plugins/lockstep.c
>>> +++ b/contrib/plugins/lockstep.c
>>> @@ -268,11 +268,13 @@ static bool setup_socket(const char *path)
>>>      socket_fd = accept(fd, NULL, NULL);
>> 
>> I think you could also simply close(fd) here instead, then you don't have to
>> do it twice below.
>> 
>
> Hi Thomas and Alex,
> Thanks for your suggestion. It's a simple and effective solution.
> Considering that the patch v3 has been queued by Alex Bennée,
> May I modify this patch and then send patch v4?

The fix has already been merged so a fresh patch would make more sense.

-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] plugins: Fix two resource leaks in connect_socket()
  2020-11-17 11:35     ` Alex Bennée
@ 2020-11-17 11:36       ` Thomas Huth
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas Huth @ 2020-11-17 11:36 UTC (permalink / raw)
  To: Alex Bennée, Alex Chen
  Cc: zhang.zhanghailiang, qemu-trivial, mjt, Laurent Vivier,
	qemu-devel, zhengchuan

On 17/11/2020 12.35, Alex Bennée wrote:
> 
> Alex Chen <alex.chen@huawei.com> writes:
> 
>> On 2020/11/17 0:50, Thomas Huth wrote:
>>> On 28/10/2020 14.45, AlexChen wrote:
>>>> Either accept() fails or exits normally, we need to close the fd.
>>>>
>>>> Reported-by: Euler Robot <euler.robot@huawei.com>
>>>> Signed-off-by: AlexChen <alex.chen@huawei.com>
>>>> ---
>>>>  contrib/plugins/lockstep.c | 2 ++
>>>>  1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c
>>>> index 319bd44b83..5aad50869d 100644
>>>> --- a/contrib/plugins/lockstep.c
>>>> +++ b/contrib/plugins/lockstep.c
>>>> @@ -268,11 +268,13 @@ static bool setup_socket(const char *path)
>>>>      socket_fd = accept(fd, NULL, NULL);
>>>
>>> I think you could also simply close(fd) here instead, then you don't have to
>>> do it twice below.
>>>
>>
>> Hi Thomas and Alex,
>> Thanks for your suggestion. It's a simple and effective solution.
>> Considering that the patch v3 has been queued by Alex Bennée,
>> May I modify this patch and then send patch v4?
> 
> The fix has already been merged so a fresh patch would make more sense.

Well, then never mind. It's not worth the code churn to just get rid of one
line of code, I think.

 Thomas



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-11-17 11:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-10-28 13:45 [PATCH 2/2] plugins: Fix two resource leaks in connect_socket() AlexChen
2020-11-05  6:54 ` AlexChen
2020-11-16 16:50 ` Thomas Huth
2020-11-17  1:13   ` Alex Chen
2020-11-17 11:35     ` Alex Bennée
2020-11-17 11:36       ` Thomas Huth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).