| Message ID | 20201029133833.3450220-4-armbru@redhat.com |
|---|---|
| State | Accepted |
| Commit | 718a9be02df880ca4b4e34ce253daf2bfc5d059c |
| Headers | show |
| Series | sockets: Attempt to drain the abstract socket swamp | expand |
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>
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 -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
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 --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
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(-)