From 5593499d4a260ac765790be8942cb631855204c4 Mon Sep 17 00:00:00 2001 From: Kyle Evans Date: Wed, 9 Sep 2020 02:42:21 +0000 Subject: [PATCH] libc/resolv: attempt to fix the test under WARNS=6 In a side-change that I'm working on to start defaulting src builds to WARNS=6 where WARNS isn't otherwise specified, GCC6 (and clang, to a lesser extent) pointed out a number of issues with the resolv tests: - Global method variable that gets shadowed in run_tests() - Signed/unsigned comparison between i in run_tests() and hosts->sl_cur The shadowed variable looks like it might actually be bogus as written, as we pass it to RUN_TESTS -> run_tests, but other parts use the global method instead. This change is mainly geared towards correcting that by removing the global and plumbing the method through from run_tests -> run into the new thread. For the signed/unsigned comparison, there's no compelling reason to not just switch i/nthreads/nhosts to size_t. The review also included a change to the load() function that was better addressed by jhb in r365302. Reviewed by: ngie, pstef MFC after: 1 week Differential Revision: https://reviews.freebsd.org/D24844 --- lib/libc/tests/resolv/resolv_test.c | 53 +++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 15 deletions(-) diff --git a/lib/libc/tests/resolv/resolv_test.c b/lib/libc/tests/resolv/resolv_test.c index 1cf27d1f32e6..12f1dca76a56 100644 --- a/lib/libc/tests/resolv/resolv_test.c +++ b/lib/libc/tests/resolv/resolv_test.c @@ -34,6 +34,8 @@ __RCSID("$NetBSD: resolv.c,v 1.6 2004/05/23 16:59:11 christos Exp $"); #include #include +#include +#include #include #include #include @@ -55,14 +57,13 @@ enum method { }; static StringList *hosts = NULL; -static enum method method = METHOD_GETADDRINFO; static int *ask = NULL; static int *got = NULL; static void load(const char *); -static void resolvone(int); +static void resolvone(int, enum method); static void *resolvloop(void *); -static void run(int *); +static void run(int *, enum method); static pthread_mutex_t stats = PTHREAD_MUTEX_INITIALIZER; @@ -173,18 +174,18 @@ resolv_getipnodeby(pthread_t self, char *host) } static void -resolvone(int n) +resolvone(int n, enum method method) { char buf[1024]; pthread_t self = pthread_self(); size_t i = (random() & 0x0fffffff) % hosts->sl_cur; char *host = hosts->sl_str[i]; - struct addrinfo hints, *res; int error, len; len = snprintf(buf, sizeof(buf), "%p: %d resolving %s %d\n", self, n, host, (int)i); (void)write(STDOUT_FILENO, buf, len); + error = 0; switch (method) { case METHOD_GETADDRINFO: error = resolv_getaddrinfo(self, host, i); @@ -196,6 +197,10 @@ resolvone(int n) error = resolv_getipnodeby(self, host); break; default: + /* UNREACHABLE */ + /* XXX Needs an __assert_unreachable() for userland. */ + assert(0 && "Unreachable segment reached"); + abort(); break; } pthread_mutex_lock(&stats); @@ -204,35 +209,53 @@ resolvone(int n) pthread_mutex_unlock(&stats); } +struct resolvloop_args { + int *nhosts; + enum method method; +}; + static void * resolvloop(void *p) { - int *nhosts = (int *)p; - if (*nhosts == 0) + struct resolvloop_args *args = p; + + if (*args->nhosts == 0) { + free(args); return NULL; + } + do - resolvone(*nhosts); - while (--(*nhosts)); + resolvone(*args->nhosts, args->method); + while (--(*args->nhosts)); + free(args); return NULL; } static void -run(int *nhosts) +run(int *nhosts, enum method method) { pthread_t self; int rc; + struct resolvloop_args *args; + /* Created thread is responsible for free(). */ + args = malloc(sizeof(*args)); + ATF_REQUIRE(args != NULL); + + args->nhosts = nhosts; + args->method = method; self = pthread_self(); - rc = pthread_create(&self, NULL, resolvloop, nhosts); + rc = pthread_create(&self, NULL, resolvloop, args); ATF_REQUIRE_MSG(rc == 0, "pthread_create failed: %s", strerror(rc)); } static int run_tests(const char *hostlist_file, enum method method) { - int nthreads = NTHREADS; - int nhosts = NHOSTS; - int i, c, done, *nleft; + size_t nthreads = NTHREADS; + size_t nhosts = NHOSTS; + size_t i; + int c, done, *nleft; hosts = sl_init(); srandom(1234); @@ -252,7 +275,7 @@ run_tests(const char *hostlist_file, enum method method) for (i = 0; i < nthreads; i++) { nleft[i] = nhosts; - run(&nleft[i]); + run(&nleft[i], method); } for (done = 0; !done;) {