diff mbox series

[03/11] test-util-sockets: Clean up SocketAddress construction

Message ID 20201029133833.3450220-4-armbru@redhat.com
State New, archived
Headers show
Series sockets: Attempt to drain the abstract socket swamp | expand

Commit Message

Markus Armbruster Oct. 29, 2020, 1:38 p.m. UTC
The thread functions build the SocketAddress from global variable
@abstract_sock_name and the tight flag passed as pointer
argument (either NULL or (gpointer)1).  There is no need for such
hackery; simply pass the SocketAddress instead.

While there, dumb down g_rand_int_range() to g_random_int().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/test-util-sockets.c | 62 +++++++++++++++------------------------
 1 file changed, 24 insertions(+), 38 deletions(-)

Comments

Eric Blake Oct. 29, 2020, 6:43 p.m. UTC | #1
On 10/29/20 8:38 AM, Markus Armbruster wrote:
> The thread functions build the SocketAddress from global variable
> @abstract_sock_name and the tight flag passed as pointer
> argument (either NULL or (gpointer)1).  There is no need for such
> hackery; simply pass the SocketAddress instead.
> 
> While there, dumb down g_rand_int_range() to g_random_int().
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  tests/test-util-sockets.c | 62 +++++++++++++++------------------------
>  1 file changed, 24 insertions(+), 38 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>
Daniel P. Berrangé Oct. 30, 2020, 9:36 a.m. UTC | #2
On Thu, Oct 29, 2020 at 02:38:25PM +0100, Markus Armbruster wrote:
> The thread functions build the SocketAddress from global variable
> @abstract_sock_name and the tight flag passed as pointer
> argument (either NULL or (gpointer)1).  There is no need for such
> hackery; simply pass the SocketAddress instead.
> 
> While there, dumb down g_rand_int_range() to g_random_int().
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  tests/test-util-sockets.c | 62 +++++++++++++++------------------------
>  1 file changed, 24 insertions(+), 38 deletions(-)
> 
> diff --git a/tests/test-util-sockets.c b/tests/test-util-sockets.c
> index 9d317e73a6..b1b5628bd5 100644
> --- a/tests/test-util-sockets.c
> +++ b/tests/test-util-sockets.c
> @@ -230,25 +230,14 @@ static void test_socket_fd_pass_num_nocli(void)
>  #endif
>  
>  #ifdef __linux__
> -static gchar *abstract_sock_name;
> -
>  static gpointer unix_server_thread_func(gpointer user_data)
>  {
> -    SocketAddress addr;

Keep this but as a pointer, and initialize it to "user_data",
so that it is clear what data type this parameter is expected
to be.

> -    Error *err = NULL;
> -    int fd = -1;
> -    int connfd = -1;
> +    int fd;
> +    int connfd;
>      struct sockaddr_un un;
>      socklen_t len = sizeof(un);
>  
> -    addr.type = SOCKET_ADDRESS_TYPE_UNIX;
> -    addr.u.q_unix.path = abstract_sock_name;
> -    addr.u.q_unix.has_tight = true;
> -    addr.u.q_unix.tight = user_data != NULL;
> -    addr.u.q_unix.has_abstract = true;
> -    addr.u.q_unix.abstract = true;
> -
> -    fd = socket_listen(&addr, 1, &err);
> +    fd = socket_listen(user_data, 1, &error_abort);

Then keep this as passing "addr"


>  
>  static gpointer unix_client_thread_func(gpointer user_data)
>  {
> -    SocketAddress addr;

Same note here

> -    Error *err = NULL;
> -    int fd = -1;
> -
> -    addr.type = SOCKET_ADDRESS_TYPE_UNIX;
> -    addr.u.q_unix.path = abstract_sock_name;
> -    addr.u.q_unix.has_tight = true;
> -    addr.u.q_unix.tight = user_data != NULL;
> -    addr.u.q_unix.has_abstract = true;
> -    addr.u.q_unix.abstract = true;
> -
> -    fd = socket_connect(&addr, &err);
> +    int fd;
>  
> +    fd = socket_connect(user_data, &error_abort);
>      g_assert_cmpint(fd, >=, 0);

Regards,
Daniel
Markus Armbruster Oct. 30, 2020, 2:06 p.m. UTC | #3
Daniel P. Berrangé <berrange@redhat.com> writes:

> On Thu, Oct 29, 2020 at 02:38:25PM +0100, Markus Armbruster wrote:
>> The thread functions build the SocketAddress from global variable
>> @abstract_sock_name and the tight flag passed as pointer
>> argument (either NULL or (gpointer)1).  There is no need for such
>> hackery; simply pass the SocketAddress instead.
>> 
>> While there, dumb down g_rand_int_range() to g_random_int().
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  tests/test-util-sockets.c | 62 +++++++++++++++------------------------
>>  1 file changed, 24 insertions(+), 38 deletions(-)
>> 
>> diff --git a/tests/test-util-sockets.c b/tests/test-util-sockets.c
>> index 9d317e73a6..b1b5628bd5 100644
>> --- a/tests/test-util-sockets.c
>> +++ b/tests/test-util-sockets.c
>> @@ -230,25 +230,14 @@ static void test_socket_fd_pass_num_nocli(void)
>>  #endif
>>  
>>  #ifdef __linux__
>> -static gchar *abstract_sock_name;
>> -
>>  static gpointer unix_server_thread_func(gpointer user_data)
>>  {
>> -    SocketAddress addr;
>
> Keep this but as a pointer, and initialize it to "user_data",
> so that it is clear what data type this parameter is expected
> to be.

Can do (I don't care for it myself).

[...]
diff mbox series

Patch

diff --git a/tests/test-util-sockets.c b/tests/test-util-sockets.c
index 9d317e73a6..b1b5628bd5 100644
--- a/tests/test-util-sockets.c
+++ b/tests/test-util-sockets.c
@@ -230,25 +230,14 @@  static void test_socket_fd_pass_num_nocli(void)
 #endif
 
 #ifdef __linux__
-static gchar *abstract_sock_name;
-
 static gpointer unix_server_thread_func(gpointer user_data)
 {
-    SocketAddress addr;
-    Error *err = NULL;
-    int fd = -1;
-    int connfd = -1;
+    int fd;
+    int connfd;
     struct sockaddr_un un;
     socklen_t len = sizeof(un);
 
-    addr.type = SOCKET_ADDRESS_TYPE_UNIX;
-    addr.u.q_unix.path = abstract_sock_name;
-    addr.u.q_unix.has_tight = true;
-    addr.u.q_unix.tight = user_data != NULL;
-    addr.u.q_unix.has_abstract = true;
-    addr.u.q_unix.abstract = true;
-
-    fd = socket_listen(&addr, 1, &err);
+    fd = socket_listen(user_data, 1, &error_abort);
     g_assert_cmpint(fd, >=, 0);
     g_assert(fd_is_socket(fd));
 
@@ -257,69 +246,66 @@  static gpointer unix_server_thread_func(gpointer user_data)
     close(connfd);
 
     close(fd);
-
     return NULL;
 }
 
 static gpointer unix_client_thread_func(gpointer user_data)
 {
-    SocketAddress addr;
-    Error *err = NULL;
-    int fd = -1;
-
-    addr.type = SOCKET_ADDRESS_TYPE_UNIX;
-    addr.u.q_unix.path = abstract_sock_name;
-    addr.u.q_unix.has_tight = true;
-    addr.u.q_unix.tight = user_data != NULL;
-    addr.u.q_unix.has_abstract = true;
-    addr.u.q_unix.abstract = true;
-
-    fd = socket_connect(&addr, &err);
+    int fd;
 
+    fd = socket_connect(user_data, &error_abort);
     g_assert_cmpint(fd, >=, 0);
-
     close(fd);
-
     return NULL;
 }
 
 static void test_socket_unix_abstract_good(void)
 {
-    GRand *r = g_rand_new();
+    SocketAddress addr;
 
-    abstract_sock_name = g_strdup_printf("unix-%d-%d", getpid(),
-                                         g_rand_int_range(r, 100, 1000));
+    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;
 
     /* non tight socklen serv and cli */
+
+    addr.u.q_unix.has_tight = false;
+    addr.u.q_unix.tight = false;
+
     GThread *serv = g_thread_new("abstract_unix_server",
                                  unix_server_thread_func,
-                                 NULL);
+                                 &addr);
 
     sleep(1);
 
     GThread *cli = g_thread_new("abstract_unix_client",
                                 unix_client_thread_func,
-                                NULL);
+                                &addr);
 
     g_thread_join(cli);
     g_thread_join(serv);
 
     /* tight socklen serv and cli */
+
+    addr.u.q_unix.has_tight = true;
+    addr.u.q_unix.tight = true;
+
     serv = g_thread_new("abstract_unix_server",
                         unix_server_thread_func,
-                        (gpointer)1);
+                        &addr);
 
     sleep(1);
 
     cli = g_thread_new("abstract_unix_client",
                        unix_client_thread_func,
-                       (gpointer)1);
+                       &addr);
 
     g_thread_join(cli);
     g_thread_join(serv);
 
-    g_free(abstract_sock_name);
-    g_rand_free(r);
+    g_free(addr.u.q_unix.path);
 }
 #endif