From b839101aacb38131f0f4313b1b76316e663e58e9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mar 23 2023 22:48:00 +0000 Subject: fileio: add new helper fdopen_independent() This is a combination of fdopen() and fd_reopen(). i.e. it first reopens the fd, and then converts that into a FILE*. We do this at various places already manually. let's move this into a helper call of its own. --- diff --git a/src/basic/env-file.c b/src/basic/env-file.c index 16de727..34027ca 100644 --- a/src/basic/env-file.c +++ b/src/basic/env-file.c @@ -381,22 +381,15 @@ int parse_env_file_fd_sentinel( const char *fname, /* only used for logging */ ...) { - _cleanup_close_ int fd_ro = -EBADF; _cleanup_fclose_ FILE *f = NULL; va_list ap; int r; assert(fd >= 0); - fd_ro = fd_reopen(fd, O_CLOEXEC | O_RDONLY); - if (fd_ro < 0) - return fd_ro; - - f = fdopen(fd_ro, "re"); - if (!f) - return -errno; - - TAKE_FD(fd_ro); + r = fdopen_independent(fd, "re", &f); + if (r < 0) + return r; va_start(ap, fname); r = parse_env_filev(f, fname, ap); diff --git a/src/basic/fileio.c b/src/basic/fileio.c index c802db7..c759815 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -1020,6 +1020,35 @@ int xfopenat(int dir_fd, const char *path, const char *mode, int flags, FILE **r return 0; } +int fdopen_independent(int fd, const char *mode, FILE **ret) { + _cleanup_close_ int copy_fd = -EBADF; + _cleanup_fclose_ FILE *f = NULL; + int mode_flags; + + assert(fd >= 0); + assert(mode); + assert(ret); + + /* A combination of fdopen() + fd_reopen(). i.e. reopens the inode the specified fd points to and + * returns a FILE* for it */ + + mode_flags = fopen_mode_to_flags(mode); + if (mode_flags < 0) + return mode_flags; + + copy_fd = fd_reopen(fd, mode_flags); + if (copy_fd < 0) + return copy_fd; + + f = fdopen(copy_fd, mode); + if (!f) + return -errno; + + TAKE_FD(copy_fd); + *ret = TAKE_PTR(f); + return 0; +} + static int search_and_fopen_internal( const char *path, const char *mode, diff --git a/src/basic/fileio.h b/src/basic/fileio.h index 7da3ee3..0a88a19 100644 --- a/src/basic/fileio.h +++ b/src/basic/fileio.h @@ -106,6 +106,8 @@ int get_proc_field(const char *filename, const char *pattern, const char *termin DIR *xopendirat(int dirfd, const char *name, int flags); int xfopenat(int dir_fd, const char *path, const char *mode, int flags, FILE **ret); +int fdopen_independent(int fd, const char *mode, FILE **ret); + int search_and_fopen(const char *path, const char *mode, const char *root, const char **search, FILE **ret, char **ret_path); int search_and_fopen_nulstr(const char *path, const char *mode, const char *root, const char *search, FILE **ret, char **ret_path); diff --git a/src/locale/localed-util.c b/src/locale/localed-util.c index d6b6593..f9eb3e7 100644 --- a/src/locale/localed-util.c +++ b/src/locale/localed-util.c @@ -312,7 +312,7 @@ int vconsole_read_data(Context *c, sd_bus_message *m) { } int x11_read_data(Context *c, sd_bus_message *m) { - _cleanup_close_ int fd = -EBADF, fd_ro = -EBADF; + _cleanup_close_ int fd = -EBADF; _cleanup_fclose_ FILE *f = NULL; bool in_section = false; struct stat st; @@ -348,15 +348,9 @@ int x11_read_data(Context *c, sd_bus_message *m) { c->x11_stat = st; x11_context_clear(&c->x11_from_xorg); - fd_ro = fd_reopen(fd, O_CLOEXEC | O_RDONLY); - if (fd_ro < 0) - return fd_ro; - - f = fdopen(fd_ro, "re"); - if (!f) - return -errno; - - TAKE_FD(fd_ro); + r = fdopen_independent(fd, "re", &f); + if (r < 0) + return r; for (;;) { _cleanup_free_ char *line = NULL; diff --git a/src/portable/portable.c b/src/portable/portable.c index 23102e5..664c873 100644 --- a/src/portable/portable.c +++ b/src/portable/portable.c @@ -594,7 +594,6 @@ static int extract_image_and_extensions( _cleanup_(portable_metadata_unrefp) PortableMetadata *extension_release_meta = NULL; _cleanup_hashmap_free_ Hashmap *extra_unit_files = NULL; _cleanup_strv_free_ char **extension_release = NULL; - _cleanup_close_ int extension_release_fd = -EBADF; _cleanup_fclose_ FILE *f = NULL; const char *e; @@ -610,11 +609,7 @@ static int extract_image_and_extensions( continue; /* We need to keep the fd valid, to return the PortableMetadata to the caller. */ - extension_release_fd = fd_reopen(extension_release_meta->fd, O_CLOEXEC|O_RDONLY); - if (extension_release_fd < 0) - return extension_release_fd; - - r = take_fdopen_unlocked(&extension_release_fd, "r", &f); + r = fdopen_independent(extension_release_meta->fd, "re", &f); if (r < 0) return r; diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c index 2e05992..1a9a8a5 100644 --- a/src/test/test-fileio.c +++ b/src/test/test-fileio.c @@ -14,6 +14,7 @@ #include "fileio.h" #include "fs-util.h" #include "io-util.h" +#include "memfd-util.h" #include "parse-util.h" #include "path-util.h" #include "process-util.h" @@ -1072,4 +1073,46 @@ TEST(test_read_virtual_file) { test_read_virtual_file_one(SIZE_MAX); } +TEST(test_fdopen_independent) { +#define TEST_TEXT "this is some random test text we are going to write to a memfd" + _cleanup_close_ int fd = -EBADF; + _cleanup_fclose_ FILE *f = NULL; + char buf[STRLEN(TEST_TEXT) + 1]; + + fd = memfd_new("fdopen_independent"); + if (fd < 0) { + assert_se(ERRNO_IS_NOT_SUPPORTED(fd)); + return; + } + + assert_se(write(fd, TEST_TEXT, strlen(TEST_TEXT)) == strlen(TEST_TEXT)); + /* we'll leave the read offset at the end of the memfd, the fdopen_independent() descriptors should + * start at the beginning anyway */ + + assert_se(fdopen_independent(fd, "re", &f) >= 0); + zero(buf); + assert_se(fread(buf, 1, sizeof(buf), f) == strlen(TEST_TEXT)); + assert_se(streq(buf, TEST_TEXT)); + assert_se((fcntl(fileno(f), F_GETFL) & O_ACCMODE) == O_RDONLY); + assert_se(FLAGS_SET(fcntl(fileno(f), F_GETFD), FD_CLOEXEC)); + f = safe_fclose(f); + + assert_se(fdopen_independent(fd, "r", &f) >= 0); + zero(buf); + assert_se(fread(buf, 1, sizeof(buf), f) == strlen(TEST_TEXT)); + assert_se(streq(buf, TEST_TEXT)); + assert_se((fcntl(fileno(f), F_GETFL) & O_ACCMODE) == O_RDONLY); + assert_se(!FLAGS_SET(fcntl(fileno(f), F_GETFD), FD_CLOEXEC)); + f = safe_fclose(f); + + assert_se(fdopen_independent(fd, "r+e", &f) >= 0); + zero(buf); + assert_se(fread(buf, 1, sizeof(buf), f) == strlen(TEST_TEXT)); + assert_se(streq(buf, TEST_TEXT)); + assert_se((fcntl(fileno(f), F_GETFL) & O_ACCMODE) == O_RDWR); + assert_se(FLAGS_SET(fcntl(fileno(f), F_GETFD), FD_CLOEXEC)); + f = safe_fclose(f); +} + + DEFINE_TEST_MAIN(LOG_DEBUG);