[PATCH] tests/unit/test-util-sockets: Use g_file_open_tmp() to create temp file

Philippe Mathieu-Daudé posted 1 patch 2 years, 4 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20211224234504.3413370-1-philmd@redhat.com
tests/unit/test-util-sockets.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
[PATCH] tests/unit/test-util-sockets: Use g_file_open_tmp() to create temp file
Posted by Philippe Mathieu-Daudé 2 years, 4 months ago
Similarly to commit e63ed64c6d1 ("tests/qtest/virtio-net-failover:
Use g_file_open_tmp() to create temporary file"), avoid calling
g_test_rand_int() before g_test_init(): use g_file_open_tmp().

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tests/unit/test-util-sockets.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/unit/test-util-sockets.c b/tests/unit/test-util-sockets.c
index 72b92465298..896247e3ed3 100644
--- a/tests/unit/test-util-sockets.c
+++ b/tests/unit/test-util-sockets.c
@@ -305,9 +305,11 @@ static void test_socket_unix_abstract(void)
     };
     int i;
 
+    i = g_file_open_tmp("unix-XXXXXX", &addr.u.q_unix.path, NULL);
+    g_assert_true(i >= 0);
+    close(i);
+
     addr.type = SOCKET_ADDRESS_TYPE_UNIX;
-    addr.u.q_unix.path = g_strdup_printf("unix-%d-%u",
-                                         getpid(), g_random_int());
     addr.u.q_unix.has_abstract = true;
     addr.u.q_unix.abstract = true;
     addr.u.q_unix.has_tight = false;
-- 
2.33.1

Re: [PATCH] tests/unit/test-util-sockets: Use g_file_open_tmp() to create temp file
Posted by Richard Henderson 2 years, 4 months ago
On 12/24/21 3:45 PM, Philippe Mathieu-Daudé wrote:
> Similarly to commit e63ed64c6d1 ("tests/qtest/virtio-net-failover:
> Use g_file_open_tmp() to create temporary file"), avoid calling
> g_test_rand_int() before g_test_init(): use g_file_open_tmp().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>   tests/unit/test-util-sockets.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/unit/test-util-sockets.c b/tests/unit/test-util-sockets.c
> index 72b92465298..896247e3ed3 100644
> --- a/tests/unit/test-util-sockets.c
> +++ b/tests/unit/test-util-sockets.c
> @@ -305,9 +305,11 @@ static void test_socket_unix_abstract(void)
>       };
>       int i;
>   
> +    i = g_file_open_tmp("unix-XXXXXX", &addr.u.q_unix.path, NULL);
> +    g_assert_true(i >= 0);

Just g_assert.  Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

> +    close(i);
> +
>       addr.type = SOCKET_ADDRESS_TYPE_UNIX;
> -    addr.u.q_unix.path = g_strdup_printf("unix-%d-%u",
> -                                         getpid(), g_random_int());
>       addr.u.q_unix.has_abstract = true;
>       addr.u.q_unix.abstract = true;
>       addr.u.q_unix.has_tight = false;
> 


Re: [PATCH] tests/unit/test-util-sockets: Use g_file_open_tmp() to create temp file
Posted by Philippe Mathieu-Daudé 2 years, 4 months ago
On 12/25/21 02:23, Richard Henderson wrote:
> On 12/24/21 3:45 PM, Philippe Mathieu-Daudé wrote:
>> Similarly to commit e63ed64c6d1 ("tests/qtest/virtio-net-failover:
>> Use g_file_open_tmp() to create temporary file"), avoid calling
>> g_test_rand_int() before g_test_init(): use g_file_open_tmp().
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>   tests/unit/test-util-sockets.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/unit/test-util-sockets.c
>> b/tests/unit/test-util-sockets.c
>> index 72b92465298..896247e3ed3 100644
>> --- a/tests/unit/test-util-sockets.c
>> +++ b/tests/unit/test-util-sockets.c
>> @@ -305,9 +305,11 @@ static void test_socket_unix_abstract(void)
>>       };
>>       int i;
>>   +    i = g_file_open_tmp("unix-XXXXXX", &addr.u.q_unix.path, NULL);
>> +    g_assert_true(i >= 0);
> 
> Just g_assert.

Per https://docs.gtk.org/glib/testing.html#testing-framework:

  Note that g_assert() should not be used in unit tests, since
  it is a no-op when compiling with G_DISABLE_ASSERT. Use g_assert()
  in production code, and g_assert_true() in unit tests.

We don't really use G_DISABLE_ASSERT, but g_assert_true()
seems appropriate here...

> Otherwise,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Thanks.