[PATCH] tests: test-qga: close socket on failure to connect

[PATCH] tests: test-qga: close socket on failure to connect

[Paolo Bonzini]

Reported by Coverity as CID 1432543.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/unit/test-qga.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/unit/test-qga.c b/tests/unit/test-qga.c
index a05a4628ed..d27ff94d13 100644
--- a/tests/unit/test-qga.c
+++ b/tests/unit/test-qga.c
@@ -32,6 +32,7 @@ static int connect_qga(char *path)
             g_usleep(G_USEC_PER_SEC);
         }
         if (i++ == 10) {
+            close(s);
             return -1;
         }
     } while (ret == -1);
-- 
2.37.2

Re: [PATCH] tests: test-qga: close socket on failure to connect

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 866 bytes --]

Hi

On Mon, Sep 5, 2022 at 3:27 PM Paolo Bonzini <pbonzini@redhat.com> wrote:

> Reported by Coverity as CID 1432543.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/unit/test-qga.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/tests/unit/test-qga.c b/tests/unit/test-qga.c
> index a05a4628ed..d27ff94d13 100644
> --- a/tests/unit/test-qga.c
> +++ b/tests/unit/test-qga.c
> @@ -32,6 +32,7 @@ static int connect_qga(char *path)
>              g_usleep(G_USEC_PER_SEC);
>          }
>          if (i++ == 10) {
> +            close(s);
>

We may want to be a bit safer and use g_close(), which handles EINTR too.
Anyway

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


>              return -1;
>          }
>      } while (ret == -1);
> --
> 2.37.2
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 1630 bytes --]

Re: [PATCH] tests: test-qga: close socket on failure to connect

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 1139 bytes --]

There is no portable way to handle EINTR on close() as far as I know. You
can't really do anything but cross your fingers.

Paolo

Il lun 5 set 2022, 13:36 Marc-André Lureau <marcandre.lureau@gmail.com> ha
scritto:

> Hi
>
> On Mon, Sep 5, 2022 at 3:27 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>> Reported by Coverity as CID 1432543.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  tests/unit/test-qga.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/tests/unit/test-qga.c b/tests/unit/test-qga.c
>> index a05a4628ed..d27ff94d13 100644
>> --- a/tests/unit/test-qga.c
>> +++ b/tests/unit/test-qga.c
>> @@ -32,6 +32,7 @@ static int connect_qga(char *path)
>>              g_usleep(G_USEC_PER_SEC);
>>          }
>>          if (i++ == 10) {
>> +            close(s);
>>
>
> We may want to be a bit safer and use g_close(), which handles EINTR too.
> Anyway
>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
>
>>              return -1;
>>          }
>>      } while (ret == -1);
>> --
>> 2.37.2
>>
>>
>>
>
> --
> Marc-André Lureau
>

[-- Attachment #2: Type: text/html, Size: 2292 bytes --]

Re: [PATCH] tests: test-qga: close socket on failure to connect

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1451 bytes --]

Hi


On Mon, Sep 5, 2022 at 4:29 PM Paolo Bonzini <pbonzini@redhat.com> wrote:

> There is no portable way to handle EINTR on close() as far as I know. You
> can't really do anything but cross your fingers.
>
>
Right, it actually ignores EINTR at this point. See:
https://gitlab.gnome.org/GNOME/glib/-/commit/f398bec5bcc0d924e2401c76a6b94133e9490835

Paolo
>
> Il lun 5 set 2022, 13:36 Marc-André Lureau <marcandre.lureau@gmail.com>
> ha scritto:
>
>> Hi
>>
>> On Mon, Sep 5, 2022 at 3:27 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>>> Reported by Coverity as CID 1432543.
>>>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>>  tests/unit/test-qga.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/tests/unit/test-qga.c b/tests/unit/test-qga.c
>>> index a05a4628ed..d27ff94d13 100644
>>> --- a/tests/unit/test-qga.c
>>> +++ b/tests/unit/test-qga.c
>>> @@ -32,6 +32,7 @@ static int connect_qga(char *path)
>>>              g_usleep(G_USEC_PER_SEC);
>>>          }
>>>          if (i++ == 10) {
>>> +            close(s);
>>>
>>
>> We may want to be a bit safer and use g_close(), which handles EINTR too.
>> Anyway
>>
>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>>
>>>              return -1;
>>>          }
>>>      } while (ret == -1);
>>> --
>>> 2.37.2
>>>
>>>
>>>
>>
>> --
>> Marc-André Lureau
>>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 3260 bytes --]