| Message ID | 20180615194705.28019-13-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fix building of tests/tcg - last chance to review! | expand |
On 15.06.2018 21:46, Alex Bennée wrote: > The fixed path and ports get in the way of running our tests and > builds in parallel. Instead of using TESTPATH we use mkdtemp() and > instead of a fixed port we allow the kernel to assign one and query it > afterwards. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > tests/tcg/multiarch/linux-test.c | 37 ++++++++++++++++---------------- > 1 file changed, 19 insertions(+), 18 deletions(-) > > diff --git a/tests/tcg/multiarch/linux-test.c b/tests/tcg/multiarch/linux-test.c > index 6f2c531474..3f73b96420 100644 > --- a/tests/tcg/multiarch/linux-test.c > +++ b/tests/tcg/multiarch/linux-test.c > @@ -41,8 +41,6 @@ > #include <setjmp.h> > #include <sys/shm.h> > > -#define TESTPATH "/tmp/linux-test.tmp" > -#define TESTPORT 7654 > #define STACK_SIZE 16384 > > static void error1(const char *filename, int line, const char *fmt, ...) > @@ -85,19 +83,15 @@ static void test_file(void) > struct iovec vecs[2]; > DIR *dir; > struct dirent *de; > + char template[] = "/tmp/linux-test-XXXXXX"; > + char *tmpdir = mkdtemp(template); > > - /* clean up, just in case */ > - unlink(TESTPATH "/file1"); > - unlink(TESTPATH "/file2"); > - unlink(TESTPATH "/file3"); > - rmdir(TESTPATH); > + chk_error(strlen(tmpdir)); That line looks wrong to me. According to my man-page of mkdtemp(), it returns either NULL or a pointer to the modified string. In case of NULL, strlen(tmpdir) will simply crash. And even if it would not crash, strlen() only returns values >= 0, so there is no way the chk_error could ever report an error here. Thomas
Hi Alex, On 06/15/2018 04:46 PM, Alex Bennée wrote: > The fixed path and ports get in the way of running our tests and > builds in parallel. Instead of using TESTPATH we use mkdtemp() and > instead of a fixed port we allow the kernel to assign one and query it > afterwards. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > tests/tcg/multiarch/linux-test.c | 37 ++++++++++++++++---------------- > 1 file changed, 19 insertions(+), 18 deletions(-) > > diff --git a/tests/tcg/multiarch/linux-test.c b/tests/tcg/multiarch/linux-test.c > index 6f2c531474..3f73b96420 100644 > --- a/tests/tcg/multiarch/linux-test.c > +++ b/tests/tcg/multiarch/linux-test.c > @@ -41,8 +41,6 @@ > #include <setjmp.h> > #include <sys/shm.h> > > -#define TESTPATH "/tmp/linux-test.tmp" > -#define TESTPORT 7654 > #define STACK_SIZE 16384 > > static void error1(const char *filename, int line, const char *fmt, ...) > @@ -85,19 +83,15 @@ static void test_file(void) > struct iovec vecs[2]; > DIR *dir; > struct dirent *de; > + char template[] = "/tmp/linux-test-XXXXXX"; Since /tmp doesn't always fit, can this be: char *tmpbase = getenv("TMPDIR"); char *template = g_strdup_printf("%s/qemu-test-XXXXXX", tmpbase ? tmpbase : "/tmp"); > + char *tmpdir = mkdtemp(template); g_free(template); > > - /* clean up, just in case */ > - unlink(TESTPATH "/file1"); > - unlink(TESTPATH "/file2"); > - unlink(TESTPATH "/file3"); > - rmdir(TESTPATH); > + chk_error(strlen(tmpdir)); > > if (getcwd(cur_dir, sizeof(cur_dir)) == NULL) > error("getcwd"); > > - chk_error(mkdir(TESTPATH, 0755)); > - > - chk_error(chdir(TESTPATH)); > + chk_error(chdir(tmpdir)); > > /* open/read/write/close/readv/writev/lseek */ > > @@ -163,7 +157,7 @@ static void test_file(void) > st.st_mtime != 1000) > error("stat time"); > > - chk_error(stat(TESTPATH, &st)); > + chk_error(stat(tmpdir, &st)); > if (!S_ISDIR(st.st_mode)) > error("stat mode"); > > @@ -185,7 +179,7 @@ static void test_file(void) > error("stat mode"); > > /* getdents */ > - dir = opendir(TESTPATH); > + dir = opendir(tmpdir); > if (!dir) > error("opendir"); > len = 0; > @@ -207,7 +201,7 @@ static void test_file(void) > chk_error(unlink("file3")); > chk_error(unlink("file2")); > chk_error(chdir(cur_dir)); > - chk_error(rmdir(TESTPATH)); > + chk_error(rmdir(tmpdir)); > } > > static void test_fork(void) > @@ -264,7 +258,7 @@ static int server_socket(void) > chk_error(setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val))); > > sockaddr.sin_family = AF_INET; > - sockaddr.sin_port = htons(TESTPORT); > + sockaddr.sin_port = htons(0); /* choose random ephemeral port) */ > sockaddr.sin_addr.s_addr = 0; > chk_error(bind(fd, (struct sockaddr *)&sockaddr, sizeof(sockaddr))); > chk_error(listen(fd, 0)); > @@ -272,7 +266,7 @@ static int server_socket(void) > > } > > -static int client_socket(void) > +static int client_socket(uint16_t port) > { > int fd; > struct sockaddr_in sockaddr; > @@ -280,7 +274,7 @@ static int client_socket(void) > /* server socket */ > fd = chk_error(socket(PF_INET, SOCK_STREAM, 0)); > sockaddr.sin_family = AF_INET; > - sockaddr.sin_port = htons(TESTPORT); > + sockaddr.sin_port = htons(port); > inet_aton("127.0.0.1", &sockaddr.sin_addr); > chk_error(connect(fd, (struct sockaddr *)&sockaddr, sizeof(sockaddr))); > return fd; > @@ -292,10 +286,17 @@ static void test_socket(void) > { > int server_fd, client_fd, fd, pid, ret, val; > struct sockaddr_in sockaddr; > - socklen_t len; > + struct sockaddr_in server_addr; > + socklen_t len, socklen; > + uint16_t server_port; > char buf[512]; > > server_fd = server_socket(); > + /* find out what port we got */ > + socklen = sizeof(server_addr); > + ret = getsockname(server_fd, &server_addr, &socklen); > + chk_error(ret); > + server_port = ntohs(server_addr.sin_port); > > /* test a few socket options */ > len = sizeof(val); > @@ -305,7 +306,7 @@ static void test_socket(void) > > pid = chk_error(fork()); > if (pid == 0) { > - client_fd = client_socket(); > + client_fd = client_socket(server_port); > send(client_fd, socket_msg, sizeof(socket_msg), 0); > close(client_fd); > exit(0); >
Philippe Mathieu-Daudé <f4bug@amsat.org> writes: > Hi Alex, > > On 06/15/2018 04:46 PM, Alex Bennée wrote: >> The fixed path and ports get in the way of running our tests and >> builds in parallel. Instead of using TESTPATH we use mkdtemp() and >> instead of a fixed port we allow the kernel to assign one and query it >> afterwards. >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> --- >> tests/tcg/multiarch/linux-test.c | 37 ++++++++++++++++---------------- >> 1 file changed, 19 insertions(+), 18 deletions(-) >> >> diff --git a/tests/tcg/multiarch/linux-test.c b/tests/tcg/multiarch/linux-test.c >> index 6f2c531474..3f73b96420 100644 >> --- a/tests/tcg/multiarch/linux-test.c >> +++ b/tests/tcg/multiarch/linux-test.c >> @@ -41,8 +41,6 @@ >> #include <setjmp.h> >> #include <sys/shm.h> >> >> -#define TESTPATH "/tmp/linux-test.tmp" >> -#define TESTPORT 7654 >> #define STACK_SIZE 16384 >> >> static void error1(const char *filename, int line, const char *fmt, ...) >> @@ -85,19 +83,15 @@ static void test_file(void) >> struct iovec vecs[2]; >> DIR *dir; >> struct dirent *de; >> + char template[] = "/tmp/linux-test-XXXXXX"; > > Since /tmp doesn't always fit, can this be: > > char *tmpbase = getenv("TMPDIR"); > char *template = g_strdup_printf("%s/qemu-test-XXXXXX", > tmpbase ? tmpbase : "/tmp"); It depends if we want to honour TMPDIR, is /tmp not likely to be there? Either way we can't use glib functions for these tests to keep the compilation simple. > >> + char *tmpdir = mkdtemp(template); > > g_free(template); > >> >> - /* clean up, just in case */ >> - unlink(TESTPATH "/file1"); >> - unlink(TESTPATH "/file2"); >> - unlink(TESTPATH "/file3"); >> - rmdir(TESTPATH); >> + chk_error(strlen(tmpdir)); >> >> if (getcwd(cur_dir, sizeof(cur_dir)) == NULL) >> error("getcwd"); >> >> - chk_error(mkdir(TESTPATH, 0755)); >> - >> - chk_error(chdir(TESTPATH)); >> + chk_error(chdir(tmpdir)); >> >> /* open/read/write/close/readv/writev/lseek */ >> >> @@ -163,7 +157,7 @@ static void test_file(void) >> st.st_mtime != 1000) >> error("stat time"); >> >> - chk_error(stat(TESTPATH, &st)); >> + chk_error(stat(tmpdir, &st)); >> if (!S_ISDIR(st.st_mode)) >> error("stat mode"); >> >> @@ -185,7 +179,7 @@ static void test_file(void) >> error("stat mode"); >> >> /* getdents */ >> - dir = opendir(TESTPATH); >> + dir = opendir(tmpdir); >> if (!dir) >> error("opendir"); >> len = 0; >> @@ -207,7 +201,7 @@ static void test_file(void) >> chk_error(unlink("file3")); >> chk_error(unlink("file2")); >> chk_error(chdir(cur_dir)); >> - chk_error(rmdir(TESTPATH)); >> + chk_error(rmdir(tmpdir)); >> } >> >> static void test_fork(void) >> @@ -264,7 +258,7 @@ static int server_socket(void) >> chk_error(setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val))); >> >> sockaddr.sin_family = AF_INET; >> - sockaddr.sin_port = htons(TESTPORT); >> + sockaddr.sin_port = htons(0); /* choose random ephemeral port) */ >> sockaddr.sin_addr.s_addr = 0; >> chk_error(bind(fd, (struct sockaddr *)&sockaddr, sizeof(sockaddr))); >> chk_error(listen(fd, 0)); >> @@ -272,7 +266,7 @@ static int server_socket(void) >> >> } >> >> -static int client_socket(void) >> +static int client_socket(uint16_t port) >> { >> int fd; >> struct sockaddr_in sockaddr; >> @@ -280,7 +274,7 @@ static int client_socket(void) >> /* server socket */ >> fd = chk_error(socket(PF_INET, SOCK_STREAM, 0)); >> sockaddr.sin_family = AF_INET; >> - sockaddr.sin_port = htons(TESTPORT); >> + sockaddr.sin_port = htons(port); >> inet_aton("127.0.0.1", &sockaddr.sin_addr); >> chk_error(connect(fd, (struct sockaddr *)&sockaddr, sizeof(sockaddr))); >> return fd; >> @@ -292,10 +286,17 @@ static void test_socket(void) >> { >> int server_fd, client_fd, fd, pid, ret, val; >> struct sockaddr_in sockaddr; >> - socklen_t len; >> + struct sockaddr_in server_addr; >> + socklen_t len, socklen; >> + uint16_t server_port; >> char buf[512]; >> >> server_fd = server_socket(); >> + /* find out what port we got */ >> + socklen = sizeof(server_addr); >> + ret = getsockname(server_fd, &server_addr, &socklen); >> + chk_error(ret); >> + server_port = ntohs(server_addr.sin_port); >> >> /* test a few socket options */ >> len = sizeof(val); >> @@ -305,7 +306,7 @@ static void test_socket(void) >> >> pid = chk_error(fork()); >> if (pid == 0) { >> - client_fd = client_socket(); >> + client_fd = client_socket(server_port); >> send(client_fd, socket_msg, sizeof(socket_msg), 0); >> close(client_fd); >> exit(0); >> -- Alex Bennée
Thomas Huth <thuth@redhat.com> writes: > On 15.06.2018 21:46, Alex Bennée wrote: >> The fixed path and ports get in the way of running our tests and >> builds in parallel. Instead of using TESTPATH we use mkdtemp() and >> instead of a fixed port we allow the kernel to assign one and query it >> afterwards. >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> --- >> tests/tcg/multiarch/linux-test.c | 37 ++++++++++++++++---------------- >> 1 file changed, 19 insertions(+), 18 deletions(-) >> >> diff --git a/tests/tcg/multiarch/linux-test.c b/tests/tcg/multiarch/linux-test.c >> index 6f2c531474..3f73b96420 100644 >> --- a/tests/tcg/multiarch/linux-test.c >> +++ b/tests/tcg/multiarch/linux-test.c >> @@ -41,8 +41,6 @@ >> #include <setjmp.h> >> #include <sys/shm.h> >> >> -#define TESTPATH "/tmp/linux-test.tmp" >> -#define TESTPORT 7654 >> #define STACK_SIZE 16384 >> >> static void error1(const char *filename, int line, const char *fmt, ...) >> @@ -85,19 +83,15 @@ static void test_file(void) >> struct iovec vecs[2]; >> DIR *dir; >> struct dirent *de; >> + char template[] = "/tmp/linux-test-XXXXXX"; >> + char *tmpdir = mkdtemp(template); >> >> - /* clean up, just in case */ >> - unlink(TESTPATH "/file1"); >> - unlink(TESTPATH "/file2"); >> - unlink(TESTPATH "/file3"); >> - rmdir(TESTPATH); >> + chk_error(strlen(tmpdir)); > > That line looks wrong to me. According to my man-page of mkdtemp(), it > returns either NULL or a pointer to the modified string. > In case of NULL, strlen(tmpdir) will simply crash. And even if it would > not crash, strlen() only returns values >= 0, so there is no way the > chk_error could ever report an error here. As we only really want to check we did actually do a mkdtemp would: chk_error(tmpdir) Be enough? > > Thomas -- Alex Bennée
On Mon, Jun 18, 2018 at 11:56:08AM +0100, Alex Bennée wrote: > > Thomas Huth <thuth@redhat.com> writes: > > > On 15.06.2018 21:46, Alex Bennée wrote: > >> The fixed path and ports get in the way of running our tests and > >> builds in parallel. Instead of using TESTPATH we use mkdtemp() and > >> instead of a fixed port we allow the kernel to assign one and query it > >> afterwards. > >> > >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > >> --- > >> tests/tcg/multiarch/linux-test.c | 37 ++++++++++++++++---------------- > >> 1 file changed, 19 insertions(+), 18 deletions(-) > >> > >> diff --git a/tests/tcg/multiarch/linux-test.c b/tests/tcg/multiarch/linux-test.c > >> index 6f2c531474..3f73b96420 100644 > >> --- a/tests/tcg/multiarch/linux-test.c > >> +++ b/tests/tcg/multiarch/linux-test.c > >> @@ -41,8 +41,6 @@ > >> #include <setjmp.h> > >> #include <sys/shm.h> > >> > >> -#define TESTPATH "/tmp/linux-test.tmp" > >> -#define TESTPORT 7654 > >> #define STACK_SIZE 16384 > >> > >> static void error1(const char *filename, int line, const char *fmt, ...) > >> @@ -85,19 +83,15 @@ static void test_file(void) > >> struct iovec vecs[2]; > >> DIR *dir; > >> struct dirent *de; > >> + char template[] = "/tmp/linux-test-XXXXXX"; > >> + char *tmpdir = mkdtemp(template); > >> > >> - /* clean up, just in case */ > >> - unlink(TESTPATH "/file1"); > >> - unlink(TESTPATH "/file2"); > >> - unlink(TESTPATH "/file3"); > >> - rmdir(TESTPATH); > >> + chk_error(strlen(tmpdir)); > > > > That line looks wrong to me. According to my man-page of mkdtemp(), it > > returns either NULL or a pointer to the modified string. > > In case of NULL, strlen(tmpdir) will simply crash. And even if it would > > not crash, strlen() only returns values >= 0, so there is no way the > > chk_error could ever report an error here. > > As we only really want to check we did actually do a mkdtemp would: > > chk_error(tmpdir) > > Be enough? I feel like this is a common task across all our test cases, so we would be well served by defining a helper program to give better semantics. I feel like we should be creating temporary files in the build dir too by default, rather than /tmp, since some of our test suites create quite large files and /tmp is a limited size RAMFS on many distros. THis leads to obscure errors when running many tests in parallel if space is exhausted. So how about creating a shared function: char *qtest_tempdir(const char *basename) { char *here = g_get_current_dir(); char *tmpl = g_strdup_printf("%s/%sXXXXXX", here, basename); g_free(here); char *res = g_mkstemp(tmpl); if (res == NULL) { error_report("Unable to create temporary dir: %s", strerror(errno)); abort(); } return res; } To be used as char *dir = qtest_tempdir("linux-test"); And all other test suites can be updated to use this over time too. 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 :|
On 18.06.2018 12:56, Alex Bennée wrote: > > Thomas Huth <thuth@redhat.com> writes: > >> On 15.06.2018 21:46, Alex Bennée wrote: >>> The fixed path and ports get in the way of running our tests and >>> builds in parallel. Instead of using TESTPATH we use mkdtemp() and >>> instead of a fixed port we allow the kernel to assign one and query it >>> afterwards. >>> >>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>> --- >>> tests/tcg/multiarch/linux-test.c | 37 ++++++++++++++++---------------- >>> 1 file changed, 19 insertions(+), 18 deletions(-) >>> >>> diff --git a/tests/tcg/multiarch/linux-test.c b/tests/tcg/multiarch/linux-test.c >>> index 6f2c531474..3f73b96420 100644 >>> --- a/tests/tcg/multiarch/linux-test.c >>> +++ b/tests/tcg/multiarch/linux-test.c >>> @@ -41,8 +41,6 @@ >>> #include <setjmp.h> >>> #include <sys/shm.h> >>> >>> -#define TESTPATH "/tmp/linux-test.tmp" >>> -#define TESTPORT 7654 >>> #define STACK_SIZE 16384 >>> >>> static void error1(const char *filename, int line, const char *fmt, ...) >>> @@ -85,19 +83,15 @@ static void test_file(void) >>> struct iovec vecs[2]; >>> DIR *dir; >>> struct dirent *de; >>> + char template[] = "/tmp/linux-test-XXXXXX"; >>> + char *tmpdir = mkdtemp(template); >>> >>> - /* clean up, just in case */ >>> - unlink(TESTPATH "/file1"); >>> - unlink(TESTPATH "/file2"); >>> - unlink(TESTPATH "/file3"); >>> - rmdir(TESTPATH); >>> + chk_error(strlen(tmpdir)); >> >> That line looks wrong to me. According to my man-page of mkdtemp(), it >> returns either NULL or a pointer to the modified string. >> In case of NULL, strlen(tmpdir) will simply crash. And even if it would >> not crash, strlen() only returns values >= 0, so there is no way the >> chk_error could ever report an error here. > > As we only really want to check we did actually do a mkdtemp would: > > chk_error(tmpdir) chk_error() checks for negative values, so I guess you rather should assert(tmpdir) here instead. Thomas
On 18.06.2018 13:08, Daniel P. Berrangé wrote: > On Mon, Jun 18, 2018 at 11:56:08AM +0100, Alex Bennée wrote: >> >> Thomas Huth <thuth@redhat.com> writes: >> >>> On 15.06.2018 21:46, Alex Bennée wrote: >>>> The fixed path and ports get in the way of running our tests and >>>> builds in parallel. Instead of using TESTPATH we use mkdtemp() and >>>> instead of a fixed port we allow the kernel to assign one and query it >>>> afterwards. >>>> >>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>>> --- >>>> tests/tcg/multiarch/linux-test.c | 37 ++++++++++++++++---------------- >>>> 1 file changed, 19 insertions(+), 18 deletions(-) >>>> >>>> diff --git a/tests/tcg/multiarch/linux-test.c b/tests/tcg/multiarch/linux-test.c >>>> index 6f2c531474..3f73b96420 100644 >>>> --- a/tests/tcg/multiarch/linux-test.c >>>> +++ b/tests/tcg/multiarch/linux-test.c >>>> @@ -41,8 +41,6 @@ >>>> #include <setjmp.h> >>>> #include <sys/shm.h> >>>> >>>> -#define TESTPATH "/tmp/linux-test.tmp" >>>> -#define TESTPORT 7654 >>>> #define STACK_SIZE 16384 >>>> >>>> static void error1(const char *filename, int line, const char *fmt, ...) >>>> @@ -85,19 +83,15 @@ static void test_file(void) >>>> struct iovec vecs[2]; >>>> DIR *dir; >>>> struct dirent *de; >>>> + char template[] = "/tmp/linux-test-XXXXXX"; >>>> + char *tmpdir = mkdtemp(template); >>>> >>>> - /* clean up, just in case */ >>>> - unlink(TESTPATH "/file1"); >>>> - unlink(TESTPATH "/file2"); >>>> - unlink(TESTPATH "/file3"); >>>> - rmdir(TESTPATH); >>>> + chk_error(strlen(tmpdir)); >>> >>> That line looks wrong to me. According to my man-page of mkdtemp(), it >>> returns either NULL or a pointer to the modified string. >>> In case of NULL, strlen(tmpdir) will simply crash. And even if it would >>> not crash, strlen() only returns values >= 0, so there is no way the >>> chk_error could ever report an error here. >> >> As we only really want to check we did actually do a mkdtemp would: >> >> chk_error(tmpdir) >> >> Be enough? > > I feel like this is a common task across all our test cases, so we would > be well served by defining a helper program to give better semantics. > > I feel like we should be creating temporary files in the build dir too > by default, rather than /tmp, since some of our test suites create quite > large files and /tmp is a limited size RAMFS on many distros. THis leads > to obscure errors when running many tests in parallel if space is exhausted. > > So how about creating a shared function: > > char *qtest_tempdir(const char *basename) { We're talking about a tcg test here, not a qtest. Thomas
On Mon, Jun 18, 2018 at 01:09:54PM +0200, Thomas Huth wrote: > On 18.06.2018 13:08, Daniel P. Berrangé wrote: > > On Mon, Jun 18, 2018 at 11:56:08AM +0100, Alex Bennée wrote: > >> > >> Thomas Huth <thuth@redhat.com> writes: > >> > >>> On 15.06.2018 21:46, Alex Bennée wrote: > >>>> The fixed path and ports get in the way of running our tests and > >>>> builds in parallel. Instead of using TESTPATH we use mkdtemp() and > >>>> instead of a fixed port we allow the kernel to assign one and query it > >>>> afterwards. > >>>> > >>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > >>>> --- > >>>> tests/tcg/multiarch/linux-test.c | 37 ++++++++++++++++---------------- > >>>> 1 file changed, 19 insertions(+), 18 deletions(-) > >>>> > >>>> diff --git a/tests/tcg/multiarch/linux-test.c b/tests/tcg/multiarch/linux-test.c > >>>> index 6f2c531474..3f73b96420 100644 > >>>> --- a/tests/tcg/multiarch/linux-test.c > >>>> +++ b/tests/tcg/multiarch/linux-test.c > >>>> @@ -41,8 +41,6 @@ > >>>> #include <setjmp.h> > >>>> #include <sys/shm.h> > >>>> > >>>> -#define TESTPATH "/tmp/linux-test.tmp" > >>>> -#define TESTPORT 7654 > >>>> #define STACK_SIZE 16384 > >>>> > >>>> static void error1(const char *filename, int line, const char *fmt, ...) > >>>> @@ -85,19 +83,15 @@ static void test_file(void) > >>>> struct iovec vecs[2]; > >>>> DIR *dir; > >>>> struct dirent *de; > >>>> + char template[] = "/tmp/linux-test-XXXXXX"; > >>>> + char *tmpdir = mkdtemp(template); > >>>> > >>>> - /* clean up, just in case */ > >>>> - unlink(TESTPATH "/file1"); > >>>> - unlink(TESTPATH "/file2"); > >>>> - unlink(TESTPATH "/file3"); > >>>> - rmdir(TESTPATH); > >>>> + chk_error(strlen(tmpdir)); > >>> > >>> That line looks wrong to me. According to my man-page of mkdtemp(), it > >>> returns either NULL or a pointer to the modified string. > >>> In case of NULL, strlen(tmpdir) will simply crash. And even if it would > >>> not crash, strlen() only returns values >= 0, so there is no way the > >>> chk_error could ever report an error here. > >> > >> As we only really want to check we did actually do a mkdtemp would: > >> > >> chk_error(tmpdir) > >> > >> Be enough? > > > > I feel like this is a common task across all our test cases, so we would > > be well served by defining a helper program to give better semantics. > > > > I feel like we should be creating temporary files in the build dir too > > by default, rather than /tmp, since some of our test suites create quite > > large files and /tmp is a limited size RAMFS on many distros. THis leads > > to obscure errors when running many tests in parallel if space is exhausted. > > > > So how about creating a shared function: > > > > char *qtest_tempdir(const char *basename) { > > We're talking about a tcg test here, not a qtest. The code I illustrated has nothing tieing it to libqtest. It can be used anywhere. libqtest simply stole the 'qtest_' naming convention. 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 Mon, Jun 18, 2018 at 11:56:08AM +0100, Alex Bennée wrote: >> >> Thomas Huth <thuth@redhat.com> writes: >> >> > On 15.06.2018 21:46, Alex Bennée wrote: >> >> The fixed path and ports get in the way of running our tests and >> >> builds in parallel. Instead of using TESTPATH we use mkdtemp() and >> >> instead of a fixed port we allow the kernel to assign one and query it >> >> afterwards. >> >> >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> >> --- >> >> tests/tcg/multiarch/linux-test.c | 37 ++++++++++++++++---------------- >> >> 1 file changed, 19 insertions(+), 18 deletions(-) >> >> >> >> diff --git a/tests/tcg/multiarch/linux-test.c b/tests/tcg/multiarch/linux-test.c >> >> index 6f2c531474..3f73b96420 100644 >> >> --- a/tests/tcg/multiarch/linux-test.c >> >> +++ b/tests/tcg/multiarch/linux-test.c >> >> @@ -41,8 +41,6 @@ >> >> #include <setjmp.h> >> >> #include <sys/shm.h> >> >> >> >> -#define TESTPATH "/tmp/linux-test.tmp" >> >> -#define TESTPORT 7654 >> >> #define STACK_SIZE 16384 >> >> >> >> static void error1(const char *filename, int line, const char *fmt, ...) >> >> @@ -85,19 +83,15 @@ static void test_file(void) >> >> struct iovec vecs[2]; >> >> DIR *dir; >> >> struct dirent *de; >> >> + char template[] = "/tmp/linux-test-XXXXXX"; >> >> + char *tmpdir = mkdtemp(template); >> >> >> >> - /* clean up, just in case */ >> >> - unlink(TESTPATH "/file1"); >> >> - unlink(TESTPATH "/file2"); >> >> - unlink(TESTPATH "/file3"); >> >> - rmdir(TESTPATH); >> >> + chk_error(strlen(tmpdir)); >> > >> > That line looks wrong to me. According to my man-page of mkdtemp(), it >> > returns either NULL or a pointer to the modified string. >> > In case of NULL, strlen(tmpdir) will simply crash. And even if it would >> > not crash, strlen() only returns values >= 0, so there is no way the >> > chk_error could ever report an error here. >> >> As we only really want to check we did actually do a mkdtemp would: >> >> chk_error(tmpdir) >> >> Be enough? > > I feel like this is a common task across all our test cases, so we would > be well served by defining a helper program to give better semantics. > > I feel like we should be creating temporary files in the build dir too > by default, rather than /tmp, since some of our test suites create quite > large files and /tmp is a limited size RAMFS on many distros. THis leads > to obscure errors when running many tests in parallel if space is exhausted. > > So how about creating a shared function: > > char *qtest_tempdir(const char *basename) { > char *here = g_get_current_dir(); > char *tmpl = g_strdup_printf("%s/%sXXXXXX", here, basename); > g_free(here); > char *res = g_mkstemp(tmpl); > if (res == NULL) { > error_report("Unable to create temporary dir: %s", > strerror(errno)); > abort(); > } > return res; > } We could do, although we can't use glib due to the potential vagaries of cross compile setups. However my personal feeling is to keep the TCG tests as self contained as possible for the time being. > > To be used as > > char *dir = qtest_tempdir("linux-test"); > > > And all other test suites can be updated to use this over time too. Is this something that could be addressed in later patches? > > Regards, > Daniel -- Alex Bennée
On 18.06.2018 14:04, Alex Bennée wrote: > > Daniel P. Berrangé <berrange@redhat.com> writes: > >> On Mon, Jun 18, 2018 at 11:56:08AM +0100, Alex Bennée wrote: >>> >>> Thomas Huth <thuth@redhat.com> writes: >>> >>>> On 15.06.2018 21:46, Alex Bennée wrote: >>>>> The fixed path and ports get in the way of running our tests and >>>>> builds in parallel. Instead of using TESTPATH we use mkdtemp() and >>>>> instead of a fixed port we allow the kernel to assign one and query it >>>>> afterwards. >>>>> >>>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>>>> --- >>>>> tests/tcg/multiarch/linux-test.c | 37 ++++++++++++++++---------------- >>>>> 1 file changed, 19 insertions(+), 18 deletions(-) >>>>> >>>>> diff --git a/tests/tcg/multiarch/linux-test.c b/tests/tcg/multiarch/linux-test.c >>>>> index 6f2c531474..3f73b96420 100644 >>>>> --- a/tests/tcg/multiarch/linux-test.c >>>>> +++ b/tests/tcg/multiarch/linux-test.c >>>>> @@ -41,8 +41,6 @@ >>>>> #include <setjmp.h> >>>>> #include <sys/shm.h> >>>>> >>>>> -#define TESTPATH "/tmp/linux-test.tmp" >>>>> -#define TESTPORT 7654 >>>>> #define STACK_SIZE 16384 >>>>> >>>>> static void error1(const char *filename, int line, const char *fmt, ...) >>>>> @@ -85,19 +83,15 @@ static void test_file(void) >>>>> struct iovec vecs[2]; >>>>> DIR *dir; >>>>> struct dirent *de; >>>>> + char template[] = "/tmp/linux-test-XXXXXX"; >>>>> + char *tmpdir = mkdtemp(template); >>>>> >>>>> - /* clean up, just in case */ >>>>> - unlink(TESTPATH "/file1"); >>>>> - unlink(TESTPATH "/file2"); >>>>> - unlink(TESTPATH "/file3"); >>>>> - rmdir(TESTPATH); >>>>> + chk_error(strlen(tmpdir)); >>>> >>>> That line looks wrong to me. According to my man-page of mkdtemp(), it >>>> returns either NULL or a pointer to the modified string. >>>> In case of NULL, strlen(tmpdir) will simply crash. And even if it would >>>> not crash, strlen() only returns values >= 0, so there is no way the >>>> chk_error could ever report an error here. >>> >>> As we only really want to check we did actually do a mkdtemp would: >>> >>> chk_error(tmpdir) >>> >>> Be enough? >> >> I feel like this is a common task across all our test cases, so we would >> be well served by defining a helper program to give better semantics. >> >> I feel like we should be creating temporary files in the build dir too >> by default, rather than /tmp, since some of our test suites create quite >> large files and /tmp is a limited size RAMFS on many distros. THis leads >> to obscure errors when running many tests in parallel if space is exhausted. >> >> So how about creating a shared function: >> >> char *qtest_tempdir(const char *basename) { >> char *here = g_get_current_dir(); >> char *tmpl = g_strdup_printf("%s/%sXXXXXX", here, basename); >> g_free(here); >> char *res = g_mkstemp(tmpl); >> if (res == NULL) { >> error_report("Unable to create temporary dir: %s", >> strerror(errno)); >> abort(); >> } >> return res; >> } > > We could do, although we can't use glib due to the potential vagaries > of cross compile setups. However my personal feeling is to keep the TCG > tests as self contained as possible for the time being. I agree. >> >> To be used as >> >> char *dir = qtest_tempdir("linux-test"); >> >> >> And all other test suites can be updated to use this over time too. > > Is this something that could be addressed in later patches? Yes, I think that's something for a later patch series. Or maybe even a BiteSizeTask? Thomas
On 06/17/2018 06:18 AM, Alex Bennée wrote: > Philippe Mathieu-Daudé <f4bug@amsat.org> writes: > >> Hi Alex, >> >> On 06/15/2018 04:46 PM, Alex Bennée wrote: >>> The fixed path and ports get in the way of running our tests and >>> builds in parallel. Instead of using TESTPATH we use mkdtemp() and >>> instead of a fixed port we allow the kernel to assign one and query it >>> afterwards. >>> >>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>> --- >>> tests/tcg/multiarch/linux-test.c | 37 ++++++++++++++++---------------- >>> 1 file changed, 19 insertions(+), 18 deletions(-) >>> >>> diff --git a/tests/tcg/multiarch/linux-test.c b/tests/tcg/multiarch/linux-test.c >>> index 6f2c531474..3f73b96420 100644 >>> --- a/tests/tcg/multiarch/linux-test.c >>> +++ b/tests/tcg/multiarch/linux-test.c >>> @@ -41,8 +41,6 @@ >>> #include <setjmp.h> >>> #include <sys/shm.h> >>> >>> -#define TESTPATH "/tmp/linux-test.tmp" >>> -#define TESTPORT 7654 >>> #define STACK_SIZE 16384 >>> >>> static void error1(const char *filename, int line, const char *fmt, ...) >>> @@ -85,19 +83,15 @@ static void test_file(void) >>> struct iovec vecs[2]; >>> DIR *dir; >>> struct dirent *de; >>> + char template[] = "/tmp/linux-test-XXXXXX"; >> >> Since /tmp doesn't always fit, can this be: >> >> char *tmpbase = getenv("TMPDIR"); >> char *template = g_strdup_printf("%s/qemu-test-XXXXXX", >> tmpbase ? tmpbase : "/tmp"); > > It depends if we want to honour TMPDIR, is /tmp not likely to be there? My /tmp is a not huge tmpfs and I had troubles running iotests which let some dangling big files on failure. Now I prefer run tests with TMPDIR=/scratch where I have plenty of slower space. Shouldn't be a problem here however. > > Either way we can't use glib functions for these tests to keep the > compilation simple. char template[PATH_MAX] + snprintf() :) > >> >>> + char *tmpdir = mkdtemp(template); >> >> g_free(template); >> >>> >>> - /* clean up, just in case */ >>> - unlink(TESTPATH "/file1"); >>> - unlink(TESTPATH "/file2"); >>> - unlink(TESTPATH "/file3"); >>> - rmdir(TESTPATH); >>> + chk_error(strlen(tmpdir)); >>> >>> if (getcwd(cur_dir, sizeof(cur_dir)) == NULL) >>> error("getcwd"); >>> >>> - chk_error(mkdir(TESTPATH, 0755)); >>> - >>> - chk_error(chdir(TESTPATH)); >>> + chk_error(chdir(tmpdir)); >>> >>> /* open/read/write/close/readv/writev/lseek */ >>> >>> @@ -163,7 +157,7 @@ static void test_file(void) >>> st.st_mtime != 1000) >>> error("stat time"); >>> >>> - chk_error(stat(TESTPATH, &st)); >>> + chk_error(stat(tmpdir, &st)); >>> if (!S_ISDIR(st.st_mode)) >>> error("stat mode"); >>> >>> @@ -185,7 +179,7 @@ static void test_file(void) >>> error("stat mode"); >>> >>> /* getdents */ >>> - dir = opendir(TESTPATH); >>> + dir = opendir(tmpdir); >>> if (!dir) >>> error("opendir"); >>> len = 0; >>> @@ -207,7 +201,7 @@ static void test_file(void) >>> chk_error(unlink("file3")); >>> chk_error(unlink("file2")); >>> chk_error(chdir(cur_dir)); >>> - chk_error(rmdir(TESTPATH)); >>> + chk_error(rmdir(tmpdir)); >>> } >>> >>> static void test_fork(void) >>> @@ -264,7 +258,7 @@ static int server_socket(void) >>> chk_error(setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val))); >>> >>> sockaddr.sin_family = AF_INET; >>> - sockaddr.sin_port = htons(TESTPORT); >>> + sockaddr.sin_port = htons(0); /* choose random ephemeral port) */ >>> sockaddr.sin_addr.s_addr = 0; >>> chk_error(bind(fd, (struct sockaddr *)&sockaddr, sizeof(sockaddr))); >>> chk_error(listen(fd, 0)); >>> @@ -272,7 +266,7 @@ static int server_socket(void) >>> >>> } >>> >>> -static int client_socket(void) >>> +static int client_socket(uint16_t port) >>> { >>> int fd; >>> struct sockaddr_in sockaddr; >>> @@ -280,7 +274,7 @@ static int client_socket(void) >>> /* server socket */ >>> fd = chk_error(socket(PF_INET, SOCK_STREAM, 0)); >>> sockaddr.sin_family = AF_INET; >>> - sockaddr.sin_port = htons(TESTPORT); >>> + sockaddr.sin_port = htons(port); >>> inet_aton("127.0.0.1", &sockaddr.sin_addr); >>> chk_error(connect(fd, (struct sockaddr *)&sockaddr, sizeof(sockaddr))); >>> return fd; >>> @@ -292,10 +286,17 @@ static void test_socket(void) >>> { >>> int server_fd, client_fd, fd, pid, ret, val; >>> struct sockaddr_in sockaddr; >>> - socklen_t len; >>> + struct sockaddr_in server_addr; >>> + socklen_t len, socklen; >>> + uint16_t server_port; >>> char buf[512]; >>> >>> server_fd = server_socket(); >>> + /* find out what port we got */ >>> + socklen = sizeof(server_addr); >>> + ret = getsockname(server_fd, &server_addr, &socklen); >>> + chk_error(ret); >>> + server_port = ntohs(server_addr.sin_port); >>> >>> /* test a few socket options */ >>> len = sizeof(val); >>> @@ -305,7 +306,7 @@ static void test_socket(void) >>> >>> pid = chk_error(fork()); >>> if (pid == 0) { >>> - client_fd = client_socket(); >>> + client_fd = client_socket(server_port); >>> send(client_fd, socket_msg, sizeof(socket_msg), 0); >>> close(client_fd); >>> exit(0); >>> > > > -- > Alex Bennée >
On 06/18/2018 12:18 PM, Philippe Mathieu-Daudé wrote: > On 06/17/2018 06:18 AM, Alex Bennée wrote: >> Philippe Mathieu-Daudé <f4bug@amsat.org> writes: >> >>> Hi Alex, >>> >>> On 06/15/2018 04:46 PM, Alex Bennée wrote: >>>> The fixed path and ports get in the way of running our tests and >>>> builds in parallel. Instead of using TESTPATH we use mkdtemp() and >>>> instead of a fixed port we allow the kernel to assign one and query it >>>> afterwards. >>>> >>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>>> --- >>>> tests/tcg/multiarch/linux-test.c | 37 ++++++++++++++++---------------- >>>> 1 file changed, 19 insertions(+), 18 deletions(-) >>>> >>>> diff --git a/tests/tcg/multiarch/linux-test.c b/tests/tcg/multiarch/linux-test.c >>>> index 6f2c531474..3f73b96420 100644 >>>> --- a/tests/tcg/multiarch/linux-test.c >>>> +++ b/tests/tcg/multiarch/linux-test.c >>>> @@ -41,8 +41,6 @@ >>>> #include <setjmp.h> >>>> #include <sys/shm.h> >>>> >>>> -#define TESTPATH "/tmp/linux-test.tmp" >>>> -#define TESTPORT 7654 >>>> #define STACK_SIZE 16384 >>>> >>>> static void error1(const char *filename, int line, const char *fmt, ...) >>>> @@ -85,19 +83,15 @@ static void test_file(void) >>>> struct iovec vecs[2]; >>>> DIR *dir; >>>> struct dirent *de; >>>> + char template[] = "/tmp/linux-test-XXXXXX"; >>> >>> Since /tmp doesn't always fit, can this be: >>> >>> char *tmpbase = getenv("TMPDIR"); >>> char *template = g_strdup_printf("%s/qemu-test-XXXXXX", >>> tmpbase ? tmpbase : "/tmp"); >> >> It depends if we want to honour TMPDIR, is /tmp not likely to be there? > > My /tmp is a not huge tmpfs and I had troubles running iotests which let > some dangling big files on failure. Now I prefer run tests with > TMPDIR=/scratch where I have plenty of slower space. > Shouldn't be a problem here however. Thus is something that could be addressed in later patch, or not. > >> >> Either way we can't use glib functions for these tests to keep the >> compilation simple. > > char template[PATH_MAX] + snprintf() :) > >> >>> >>>> + char *tmpdir = mkdtemp(template); >>> >>> g_free(template); >>> >>>> >>>> - /* clean up, just in case */ >>>> - unlink(TESTPATH "/file1"); >>>> - unlink(TESTPATH "/file2"); >>>> - unlink(TESTPATH "/file3"); >>>> - rmdir(TESTPATH); >>>> + chk_error(strlen(tmpdir)); >>>> >>>> if (getcwd(cur_dir, sizeof(cur_dir)) == NULL) >>>> error("getcwd"); >>>> >>>> - chk_error(mkdir(TESTPATH, 0755)); >>>> - >>>> - chk_error(chdir(TESTPATH)); >>>> + chk_error(chdir(tmpdir)); >>>> >>>> /* open/read/write/close/readv/writev/lseek */ >>>> >>>> @@ -163,7 +157,7 @@ static void test_file(void) >>>> st.st_mtime != 1000) >>>> error("stat time"); >>>> >>>> - chk_error(stat(TESTPATH, &st)); >>>> + chk_error(stat(tmpdir, &st)); >>>> if (!S_ISDIR(st.st_mode)) >>>> error("stat mode"); >>>> >>>> @@ -185,7 +179,7 @@ static void test_file(void) >>>> error("stat mode"); >>>> >>>> /* getdents */ >>>> - dir = opendir(TESTPATH); >>>> + dir = opendir(tmpdir); >>>> if (!dir) >>>> error("opendir"); >>>> len = 0; >>>> @@ -207,7 +201,7 @@ static void test_file(void) >>>> chk_error(unlink("file3")); >>>> chk_error(unlink("file2")); >>>> chk_error(chdir(cur_dir)); >>>> - chk_error(rmdir(TESTPATH)); >>>> + chk_error(rmdir(tmpdir)); >>>> } >>>> >>>> static void test_fork(void) >>>> @@ -264,7 +258,7 @@ static int server_socket(void) >>>> chk_error(setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val))); >>>> >>>> sockaddr.sin_family = AF_INET; >>>> - sockaddr.sin_port = htons(TESTPORT); >>>> + sockaddr.sin_port = htons(0); /* choose random ephemeral port) */ >>>> sockaddr.sin_addr.s_addr = 0; >>>> chk_error(bind(fd, (struct sockaddr *)&sockaddr, sizeof(sockaddr))); >>>> chk_error(listen(fd, 0)); >>>> @@ -272,7 +266,7 @@ static int server_socket(void) >>>> >>>> } >>>> >>>> -static int client_socket(void) >>>> +static int client_socket(uint16_t port) >>>> { >>>> int fd; >>>> struct sockaddr_in sockaddr; >>>> @@ -280,7 +274,7 @@ static int client_socket(void) >>>> /* server socket */ >>>> fd = chk_error(socket(PF_INET, SOCK_STREAM, 0)); >>>> sockaddr.sin_family = AF_INET; >>>> - sockaddr.sin_port = htons(TESTPORT); >>>> + sockaddr.sin_port = htons(port); >>>> inet_aton("127.0.0.1", &sockaddr.sin_addr); >>>> chk_error(connect(fd, (struct sockaddr *)&sockaddr, sizeof(sockaddr))); >>>> return fd; >>>> @@ -292,10 +286,17 @@ static void test_socket(void) >>>> { >>>> int server_fd, client_fd, fd, pid, ret, val; >>>> struct sockaddr_in sockaddr; >>>> - socklen_t len; >>>> + struct sockaddr_in server_addr; >>>> + socklen_t len, socklen; >>>> + uint16_t server_port; >>>> char buf[512]; >>>> >>>> server_fd = server_socket(); >>>> + /* find out what port we got */ >>>> + socklen = sizeof(server_addr); >>>> + ret = getsockname(server_fd, &server_addr, &socklen); >>>> + chk_error(ret); >>>> + server_port = ntohs(server_addr.sin_port); >>>> >>>> /* test a few socket options */ >>>> len = sizeof(val); >>>> @@ -305,7 +306,7 @@ static void test_socket(void) >>>> >>>> pid = chk_error(fork()); >>>> if (pid == 0) { >>>> - client_fd = client_socket(); >>>> + client_fd = client_socket(server_port); >>>> send(client_fd, socket_msg, sizeof(socket_msg), 0); >>>> close(client_fd); >>>> exit(0); >>>> >> >> >> -- >> Alex Bennée >>
diff --git a/tests/tcg/multiarch/linux-test.c b/tests/tcg/multiarch/linux-test.c index 6f2c531474..3f73b96420 100644 --- a/tests/tcg/multiarch/linux-test.c +++ b/tests/tcg/multiarch/linux-test.c @@ -41,8 +41,6 @@ #include <setjmp.h> #include <sys/shm.h> -#define TESTPATH "/tmp/linux-test.tmp" -#define TESTPORT 7654 #define STACK_SIZE 16384 static void error1(const char *filename, int line, const char *fmt, ...) @@ -85,19 +83,15 @@ static void test_file(void) struct iovec vecs[2]; DIR *dir; struct dirent *de; + char template[] = "/tmp/linux-test-XXXXXX"; + char *tmpdir = mkdtemp(template); - /* clean up, just in case */ - unlink(TESTPATH "/file1"); - unlink(TESTPATH "/file2"); - unlink(TESTPATH "/file3"); - rmdir(TESTPATH); + chk_error(strlen(tmpdir)); if (getcwd(cur_dir, sizeof(cur_dir)) == NULL) error("getcwd"); - chk_error(mkdir(TESTPATH, 0755)); - - chk_error(chdir(TESTPATH)); + chk_error(chdir(tmpdir)); /* open/read/write/close/readv/writev/lseek */ @@ -163,7 +157,7 @@ static void test_file(void) st.st_mtime != 1000) error("stat time"); - chk_error(stat(TESTPATH, &st)); + chk_error(stat(tmpdir, &st)); if (!S_ISDIR(st.st_mode)) error("stat mode"); @@ -185,7 +179,7 @@ static void test_file(void) error("stat mode"); /* getdents */ - dir = opendir(TESTPATH); + dir = opendir(tmpdir); if (!dir) error("opendir"); len = 0; @@ -207,7 +201,7 @@ static void test_file(void) chk_error(unlink("file3")); chk_error(unlink("file2")); chk_error(chdir(cur_dir)); - chk_error(rmdir(TESTPATH)); + chk_error(rmdir(tmpdir)); } static void test_fork(void) @@ -264,7 +258,7 @@ static int server_socket(void) chk_error(setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val))); sockaddr.sin_family = AF_INET; - sockaddr.sin_port = htons(TESTPORT); + sockaddr.sin_port = htons(0); /* choose random ephemeral port) */ sockaddr.sin_addr.s_addr = 0; chk_error(bind(fd, (struct sockaddr *)&sockaddr, sizeof(sockaddr))); chk_error(listen(fd, 0)); @@ -272,7 +266,7 @@ static int server_socket(void) } -static int client_socket(void) +static int client_socket(uint16_t port) { int fd; struct sockaddr_in sockaddr; @@ -280,7 +274,7 @@ static int client_socket(void) /* server socket */ fd = chk_error(socket(PF_INET, SOCK_STREAM, 0)); sockaddr.sin_family = AF_INET; - sockaddr.sin_port = htons(TESTPORT); + sockaddr.sin_port = htons(port); inet_aton("127.0.0.1", &sockaddr.sin_addr); chk_error(connect(fd, (struct sockaddr *)&sockaddr, sizeof(sockaddr))); return fd; @@ -292,10 +286,17 @@ static void test_socket(void) { int server_fd, client_fd, fd, pid, ret, val; struct sockaddr_in sockaddr; - socklen_t len; + struct sockaddr_in server_addr; + socklen_t len, socklen; + uint16_t server_port; char buf[512]; server_fd = server_socket(); + /* find out what port we got */ + socklen = sizeof(server_addr); + ret = getsockname(server_fd, &server_addr, &socklen); + chk_error(ret); + server_port = ntohs(server_addr.sin_port); /* test a few socket options */ len = sizeof(val); @@ -305,7 +306,7 @@ static void test_socket(void) pid = chk_error(fork()); if (pid == 0) { - client_fd = client_socket(); + client_fd = client_socket(server_port); send(client_fd, socket_msg, sizeof(socket_msg), 0); close(client_fd); exit(0);
The fixed path and ports get in the way of running our tests and builds in parallel. Instead of using TESTPATH we use mkdtemp() and instead of a fixed port we allow the kernel to assign one and query it afterwards. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- tests/tcg/multiarch/linux-test.c | 37 ++++++++++++++++---------------- 1 file changed, 19 insertions(+), 18 deletions(-) -- 2.17.1