From 627d2bac2477986401400127fb31af33ca0f69b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 16 May 2018 11:35:41 +0200 Subject: [PATCH] fs-util,test: add helper to remove tempfiles This simplifies the use of tempfiles in tests and fixes "leaked" temporary files in test-fileio, test-catalog, test-conf-parser. Not the whole tree is converted. --- src/basic/fs-util.c | 9 ++++++ src/basic/fs-util.h | 1 + src/journal/test-catalog.c | 29 ++++++----------- src/journal/test-compress.c | 9 +++--- src/test/test-conf-parser.c | 3 +- src/test/test-fileio.c | 64 +++++++++++++------------------------ src/test/test-unit-file.c | 16 ++++------ 7 files changed, 54 insertions(+), 77 deletions(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 232a21c193..7d788515cc 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -1066,6 +1066,15 @@ int access_fd(int fd, int mode) { return r; } +void unlink_tempfilep(char (*p)[]) { + /* If the file is created with mkstemp(), it will (almost always) + * change the suffix. Treat this as a sign that the file was + * successfully created. We ignore both the rare case where the + * original suffix is used and unlink failures. */ + if (!endswith(*p, ".XXXXXX")) + (void) unlink(*p); +} + int unlinkat_deallocate(int fd, const char *name, int flags) { _cleanup_close_ int truncate_fd = -1; struct stat st; diff --git a/src/basic/fs-util.h b/src/basic/fs-util.h index 6157fe81bf..4923a30074 100644 --- a/src/basic/fs-util.h +++ b/src/basic/fs-util.h @@ -104,6 +104,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(char*, unlink_and_free); int access_fd(int fd, int mode); +void unlink_tempfilep(char (*p)[]); int unlinkat_deallocate(int fd, const char *name, int flags); int fsync_directory_of_file(int fd); diff --git a/src/journal/test-catalog.c b/src/journal/test-catalog.c index d5640c549c..67466cf326 100644 --- a/src/journal/test-catalog.c +++ b/src/journal/test-catalog.c @@ -16,6 +16,7 @@ #include "alloc-util.h" #include "catalog.h" #include "fd-util.h" +#include "fs-util.h" #include "fileio.h" #include "log.h" #include "macro.h" @@ -32,9 +33,8 @@ static const char *no_catalog_dirs[] = { NULL }; -static Hashmap * test_import(const char* contents, ssize_t size, int code) { - int r; - char name[] = "/tmp/test-catalog.XXXXXX"; +static Hashmap* test_import(const char* contents, ssize_t size, int code) { + _cleanup_(unlink_tempfilep) char name[] = "/tmp/test-catalog.XXXXXX"; _cleanup_close_ int fd; Hashmap *h; @@ -47,10 +47,7 @@ static Hashmap * test_import(const char* contents, ssize_t size, int code) { assert_se(fd >= 0); assert_se(write(fd, contents, size) == size); - r = catalog_import_file(h, name); - assert_se(r == code); - - unlink(name); + assert_se(catalog_import_file(h, name) == code); return h; } @@ -164,17 +161,9 @@ static void test_catalog_import_merge_no_body(void) { } } -static const char* database = NULL; - -static void test_catalog_update(void) { - static char name[] = "/tmp/test-catalog.XXXXXX"; +static void test_catalog_update(const char *database) { int r; - r = mkostemp_safe(name); - assert_se(r >= 0); - - database = name; - /* Test what happens if there are no files. */ r = catalog_update(database, NULL, NULL); assert_se(r == 0); @@ -218,6 +207,7 @@ static void test_catalog_file_lang(void) { } int main(int argc, char *argv[]) { + _cleanup_(unlink_tempfilep) char database[] = "/tmp/test-catalog.XXXXXX"; _cleanup_free_ char *text = NULL; int r; @@ -234,7 +224,9 @@ int main(int argc, char *argv[]) { test_catalog_import_merge(); test_catalog_import_merge_no_body(); - test_catalog_update(); + assert_se(mkostemp_safe(database) >= 0); + + test_catalog_update(database); r = catalog_list(stdout, database, true); assert_se(r >= 0); @@ -245,8 +237,5 @@ int main(int argc, char *argv[]) { assert_se(catalog_get(database, SD_MESSAGE_COREDUMP, &text) >= 0); printf(">>>%s<<<\n", text); - if (database) - unlink(database); - return 0; } diff --git a/src/journal/test-compress.c b/src/journal/test-compress.c index 1fb8a3e2fb..f27a1f88db 100644 --- a/src/journal/test-compress.c +++ b/src/journal/test-compress.c @@ -13,6 +13,7 @@ #include "compress.h" #include "fd-util.h" #include "fileio.h" +#include "fs-util.h" #include "macro.h" #include "path-util.h" #include "random-util.h" @@ -142,8 +143,9 @@ static void test_compress_stream(int compression, const char *srcfile) { _cleanup_close_ int src = -1, dst = -1, dst2 = -1; - char pattern[] = "/tmp/systemd-test.compressed.XXXXXX", - pattern2[] = "/tmp/systemd-test.compressed.XXXXXX"; + _cleanup_(unlink_tempfilep) char + pattern[] = "/tmp/systemd-test.compressed.XXXXXX", + pattern2[] = "/tmp/systemd-test.compressed.XXXXXX"; int r; _cleanup_free_ char *cmd = NULL, *cmd2 = NULL; struct stat st = {}; @@ -195,9 +197,6 @@ static void test_compress_stream(int compression, assert_se(lseek(dst2, 0, SEEK_SET) == 0); r = decompress(dst, dst2, st.st_size - 1); assert_se(r == -EFBIG); - - assert_se(unlink(pattern) == 0); - assert_se(unlink(pattern2) == 0); } #endif diff --git a/src/test/test-conf-parser.c b/src/test/test-conf-parser.c index 90ec20378f..d569bdd85f 100644 --- a/src/test/test-conf-parser.c +++ b/src/test/test-conf-parser.c @@ -8,6 +8,7 @@ #include "conf-parser.h" #include "fd-util.h" #include "fileio.h" +#include "fs-util.h" #include "log.h" #include "macro.h" #include "string-util.h" @@ -313,7 +314,7 @@ static const char* const config_file[] = { }; static void test_config_parse(unsigned i, const char *s) { - char name[] = "/tmp/test-conf-parser.XXXXXX"; + _cleanup_(unlink_tempfilep) char name[] = "/tmp/test-conf-parser.XXXXXX"; int fd, r; _cleanup_fclose_ FILE *f = NULL; _cleanup_free_ char *setting1 = NULL; diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c index ba22e865fd..50d1e792c0 100644 --- a/src/test/test-fileio.c +++ b/src/test/test-fileio.c @@ -15,6 +15,7 @@ #include "env-util.h" #include "fd-util.h" #include "fileio.h" +#include "fs-util.h" #include "io-util.h" #include "parse-util.h" #include "process-util.h" @@ -23,7 +24,8 @@ #include "util.h" static void test_parse_env_file(void) { - char t[] = "/tmp/test-fileio-in-XXXXXX", + _cleanup_(unlink_tempfilep) char + t[] = "/tmp/test-fileio-in-XXXXXX", p[] = "/tmp/test-fileio-out-XXXXXX"; int fd, r; FILE *f; @@ -135,13 +137,11 @@ static void test_parse_env_file(void) { r = load_env_file(NULL, p, NULL, &b); assert_se(r >= 0); - - unlink(t); - unlink(p); } static void test_parse_multiline_env_file(void) { - char t[] = "/tmp/test-fileio-in-XXXXXX", + _cleanup_(unlink_tempfilep) char + t[] = "/tmp/test-fileio-in-XXXXXX", p[] = "/tmp/test-fileio-out-XXXXXX"; int fd, r; FILE *f; @@ -189,13 +189,10 @@ static void test_parse_multiline_env_file(void) { r = load_env_file(NULL, p, NULL, &b); assert_se(r >= 0); - - unlink(t); - unlink(p); } static void test_merge_env_file(void) { - char t[] = "/tmp/test-fileio-XXXXXX"; + _cleanup_(unlink_tempfilep) char t[] = "/tmp/test-fileio-XXXXXX"; int fd, r; _cleanup_fclose_ FILE *f = NULL; _cleanup_strv_free_ char **a = NULL; @@ -264,7 +261,7 @@ static void test_merge_env_file(void) { } static void test_merge_env_file_invalid(void) { - char t[] = "/tmp/test-fileio-XXXXXX"; + _cleanup_(unlink_tempfilep) char t[] = "/tmp/test-fileio-XXXXXX"; int fd, r; _cleanup_fclose_ FILE *f = NULL; _cleanup_strv_free_ char **a = NULL; @@ -303,9 +300,9 @@ static void test_merge_env_file_invalid(void) { } static void test_executable_is_script(void) { - char t[] = "/tmp/test-executable-XXXXXX"; + _cleanup_(unlink_tempfilep) char t[] = "/tmp/test-fileio-XXXXXX"; int fd, r; - FILE *f; + _cleanup_fclose_ FILE *f = NULL; char *command; fd = mkostemp_safe(t); @@ -331,9 +328,6 @@ static void test_executable_is_script(void) { assert_se(startswith(command, "/")); free(command); } - - fclose(f); - unlink(t); } static void test_status_field(void) { @@ -392,8 +386,8 @@ static void test_capeff(void) { } static void test_write_string_stream(void) { - char fn[] = "/tmp/test-write_string_stream-XXXXXX"; - FILE *f = NULL; + _cleanup_(unlink_tempfilep) char fn[] = "/tmp/test-write_string_stream-XXXXXX"; + _cleanup_fclose_ FILE *f = NULL; int fd; char buf[64]; @@ -424,13 +418,10 @@ static void test_write_string_stream(void) { assert_se(fgets(buf, sizeof(buf), f)); printf(">%s<", buf); assert_se(streq(buf, "boohoo")); - f = safe_fclose(f); - - unlink(fn); } static void test_write_string_file(void) { - char fn[] = "/tmp/test-write_string_file-XXXXXX"; + _cleanup_(unlink_tempfilep) char fn[] = "/tmp/test-write_string_file-XXXXXX"; char buf[64] = {}; _cleanup_close_ int fd; @@ -441,12 +432,10 @@ static void test_write_string_file(void) { assert_se(read(fd, buf, sizeof(buf)) == 7); assert_se(streq(buf, "boohoo\n")); - - unlink(fn); } static void test_write_string_file_no_create(void) { - char fn[] = "/tmp/test-write_string_file_no_create-XXXXXX"; + _cleanup_(unlink_tempfilep) char fn[] = "/tmp/test-write_string_file_no_create-XXXXXX"; _cleanup_close_ int fd; char buf[64] = {0}; @@ -458,8 +447,6 @@ static void test_write_string_file_no_create(void) { assert_se(read(fd, buf, sizeof(buf)) == STRLEN("boohoo\n")); assert_se(streq(buf, "boohoo\n")); - - unlink(fn); } static void test_write_string_file_verify(void) { @@ -483,9 +470,8 @@ static void test_write_string_file_verify(void) { } static void test_load_env_file_pairs(void) { - char fn[] = "/tmp/test-load_env_file_pairs-XXXXXX"; - int fd; - int r; + _cleanup_(unlink_tempfilep) char fn[] = "/tmp/test-load_env_file_pairs-XXXXXX"; + int fd, r; _cleanup_fclose_ FILE *f = NULL; _cleanup_strv_free_ char **l = NULL; char **k, **v; @@ -522,15 +508,13 @@ static void test_load_env_file_pairs(void) { if (streq(*k, "SUPPORT_URL")) assert_se(streq(*v, "https://bbs.archlinux.org/")); if (streq(*k, "BUG_REPORT_URL")) assert_se(streq(*v, "https://bugs.archlinux.org/")); } - - unlink(fn); } static void test_search_and_fopen(void) { const char *dirs[] = {"/tmp/foo/bar", "/tmp", NULL}; + char name[] = "/tmp/test-search_and_fopen.XXXXXX"; - int fd = -1; - int r; + int fd, r; FILE *f; fd = mkostemp_safe(name); @@ -563,9 +547,9 @@ static void test_search_and_fopen(void) { static void test_search_and_fopen_nulstr(void) { const char dirs[] = "/tmp/foo/bar\0/tmp\0"; - char name[] = "/tmp/test-search_and_fopen.XXXXXX"; - int fd = -1; - int r; + + _cleanup_(unlink_tempfilep) char name[] = "/tmp/test-search_and_fopen.XXXXXX"; + int fd, r; FILE *f; fd = mkostemp_safe(name); @@ -593,12 +577,12 @@ static void test_search_and_fopen_nulstr(void) { } static void test_writing_tmpfile(void) { - char name[] = "/tmp/test-systemd_writing_tmpfile.XXXXXX"; + _cleanup_(unlink_tempfilep) char name[] = "/tmp/test-systemd_writing_tmpfile.XXXXXX"; _cleanup_free_ char *contents = NULL; size_t size; - int r; _cleanup_close_ int fd = -1; struct iovec iov[3]; + int r; iov[0] = IOVEC_MAKE_STRING("abc\n"); iov[1] = IOVEC_MAKE_STRING(ALPHANUMERICAL "\n"); @@ -614,8 +598,6 @@ static void test_writing_tmpfile(void) { assert_se(r == 0); printf("contents: %s", contents); assert_se(streq(contents, "abc\n" ALPHANUMERICAL "\n")); - - unlink(name); } static void test_tempfn(void) { @@ -703,7 +685,7 @@ static void test_read_line(void) { } static void test_read_line2(void) { - char name[] = "/tmp/test-fileio.XXXXXX"; + _cleanup_(unlink_tempfilep) char name[] = "/tmp/test-fileio.XXXXXX"; int fd; _cleanup_fclose_ FILE *f = NULL; diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c index d6dec64fb8..fb4da53027 100644 --- a/src/test/test-unit-file.c +++ b/src/test/test-unit-file.c @@ -19,6 +19,7 @@ #include "conf-parser.h" #include "fd-util.h" #include "fileio.h" +#include "fs-util.h" #include "hashmap.h" #include "hostname-util.h" #include "install-printf.h" @@ -532,7 +533,7 @@ static void test_load_env_file_1(void) { _cleanup_strv_free_ char **data = NULL; int r; - char name[] = "/tmp/test-load-env-file.XXXXXX"; + _cleanup_(unlink_tempfilep) char name[] = "/tmp/test-load-env-file.XXXXXX"; _cleanup_close_ int fd; fd = mkostemp_safe(name); @@ -548,14 +549,13 @@ static void test_load_env_file_1(void) { assert_se(streq(data[4], "h=h")); assert_se(streq(data[5], "i=i")); assert_se(data[6] == NULL); - unlink(name); } static void test_load_env_file_2(void) { _cleanup_strv_free_ char **data = NULL; int r; - char name[] = "/tmp/test-load-env-file.XXXXXX"; + _cleanup_(unlink_tempfilep) char name[] = "/tmp/test-load-env-file.XXXXXX"; _cleanup_close_ int fd; fd = mkostemp_safe(name); @@ -566,14 +566,13 @@ static void test_load_env_file_2(void) { assert_se(r == 0); assert_se(streq(data[0], "a=a")); assert_se(data[1] == NULL); - unlink(name); } static void test_load_env_file_3(void) { _cleanup_strv_free_ char **data = NULL; int r; - char name[] = "/tmp/test-load-env-file.XXXXXX"; + _cleanup_(unlink_tempfilep) char name[] = "/tmp/test-load-env-file.XXXXXX"; _cleanup_close_ int fd; fd = mkostemp_safe(name); @@ -583,12 +582,11 @@ static void test_load_env_file_3(void) { r = load_env_file(NULL, name, NULL, &data); assert_se(r == 0); assert_se(data == NULL); - unlink(name); } static void test_load_env_file_4(void) { _cleanup_strv_free_ char **data = NULL; - char name[] = "/tmp/test-load-env-file.XXXXXX"; + _cleanup_(unlink_tempfilep) char name[] = "/tmp/test-load-env-file.XXXXXX"; _cleanup_close_ int fd; int r; @@ -602,14 +600,13 @@ static void test_load_env_file_4(void) { assert_se(streq(data[1], "MODULE_0=coretemp")); assert_se(streq(data[2], "MODULE_1=f71882fg")); assert_se(data[3] == NULL); - unlink(name); } static void test_load_env_file_5(void) { _cleanup_strv_free_ char **data = NULL; int r; - char name[] = "/tmp/test-load-env-file.XXXXXX"; + _cleanup_(unlink_tempfilep) char name[] = "/tmp/test-load-env-file.XXXXXX"; _cleanup_close_ int fd; fd = mkostemp_safe(name); @@ -621,7 +618,6 @@ static void test_load_env_file_5(void) { assert_se(streq(data[0], "a=")); assert_se(streq(data[1], "b=")); assert_se(data[2] == NULL); - unlink(name); } static void test_install_printf(void) {