On Mon, Jun 18, 2018 at 11:56:08AM +0100, Alex Bennée wrote: > > Thomas Huth <th...@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.ben...@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 :|