From 74ca738f6a01fb5fc19c5c3899f5cb1fdc1d7f68 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Sep 2015 15:20:10 +0200 Subject: [PATCH] util: introduce safe_fclose() and port everything over to it Adds a coccinelle script to port things over automatically. --- coccinelle/safe_fclose.cocci | 27 ++++++++++++++++++++++++++ src/basic/util.c | 27 ++++++++++++++++++++++++++ src/basic/util.h | 8 +++++++- src/bootchart/bootchart.c | 5 +---- src/core/main.c | 21 +++++--------------- src/core/mount.c | 5 +---- src/core/swap.c | 5 +---- src/dbus1-generator/dbus1-generator.c | 3 +-- src/journal-remote/journal-gatewayd.c | 3 +-- src/journal/coredump.c | 3 +-- src/journal/stacktrace.c | 6 ++---- src/libsystemd/sd-bus/bus-introspect.c | 3 +-- src/libsystemd/sd-hwdb/sd-hwdb.c | 3 +-- src/shared/pager.c | 9 +++------ 14 files changed, 79 insertions(+), 49 deletions(-) create mode 100644 coccinelle/safe_fclose.cocci diff --git a/coccinelle/safe_fclose.cocci b/coccinelle/safe_fclose.cocci new file mode 100644 index 0000000000..6961cd0164 --- /dev/null +++ b/coccinelle/safe_fclose.cocci @@ -0,0 +1,27 @@ +@@ +expression p; +@@ +- if (p) { +- fclose(p); +- p = NULL; +- } ++ p = safe_fclose(p); +@@ +expression p; +@@ +- if (p) +- fclose(p); +- p = NULL; ++ p = safe_fclose(p); +@@ +expression p; +@@ +- fclose(p); +- p = NULL; ++ p = safe_fclose(p); +@@ +expression p; +@@ +- if (p) +- fclose(p); ++ safe_fclose(p); diff --git a/src/basic/util.c b/src/basic/util.c index 3e41e6ad41..f7b2edf88c 100644 --- a/src/basic/util.c +++ b/src/basic/util.c @@ -327,6 +327,33 @@ void close_many(const int fds[], unsigned n_fd) { safe_close(fds[i]); } +int fclose_nointr(FILE *f) { + assert(f); + + /* Same as close_nointr(), but for fclose() */ + + if (fclose(f) == 0) + return 0; + + if (errno == EINTR) + return 0; + + return -errno; +} + +FILE* safe_fclose(FILE *f) { + + /* Same as safe_close(), but for fclose() */ + + if (f) { + PROTECT_ERRNO; + + assert_se(fclose_nointr(f) != EBADF); + } + + return NULL; +} + int unlink_noerrno(const char *path) { PROTECT_ERRNO; int r; diff --git a/src/basic/util.h b/src/basic/util.h index 0fafebd52d..48567756b9 100644 --- a/src/basic/util.h +++ b/src/basic/util.h @@ -149,6 +149,9 @@ void safe_close_pair(int p[]); void close_many(const int fds[], unsigned n_fd); +int fclose_nointr(FILE *f); +FILE* safe_fclose(FILE *f); + int parse_size(const char *t, off_t base, off_t *size); int parse_boolean(const char *v) _pure_; @@ -514,7 +517,10 @@ static inline void close_pairp(int (*p)[2]) { safe_close_pair(*p); } -DEFINE_TRIVIAL_CLEANUP_FUNC(FILE*, fclose); +static inline void fclosep(FILE **f) { + safe_fclose(*f); +} + DEFINE_TRIVIAL_CLEANUP_FUNC(FILE*, pclose); DEFINE_TRIVIAL_CLEANUP_FUNC(DIR*, closedir); DEFINE_TRIVIAL_CLEANUP_FUNC(FILE*, endmntent); diff --git a/src/bootchart/bootchart.c b/src/bootchart/bootchart.c index e61b58b5e5..14b630c6f9 100644 --- a/src/bootchart/bootchart.c +++ b/src/bootchart/bootchart.c @@ -458,10 +458,7 @@ int main(int argc, char *argv[]) { ps = ps->next_ps; ps->schedstat = safe_close(ps->schedstat); ps->sched = safe_close(ps->sched); - if (ps->smaps) { - fclose(ps->smaps); - ps->smaps = NULL; - } + ps->smaps = safe_fclose(ps->smaps); } if (!of) { diff --git a/src/core/main.c b/src/core/main.c index 9aaf244e71..6577a06621 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -918,8 +918,7 @@ static int parse_argv(int argc, char *argv[]) { if (!f) return log_error_errno(errno, "Failed to open serialization fd: %m"); - if (arg_serialization) - fclose(arg_serialization); + safe_fclose(arg_serialization); arg_serialization = f; @@ -1059,8 +1058,7 @@ static int prepare_reexecute(Manager *m, FILE **_f, FDSet **_fds, bool switching fail: fdset_free(fds); - if (f) - fclose(f); + safe_fclose(f); return r; } @@ -1678,10 +1676,7 @@ int main(int argc, char *argv[]) { fdset_free(fds); fds = NULL; - if (arg_serialization) { - fclose(arg_serialization); - arg_serialization = NULL; - } + arg_serialization = safe_fclose(arg_serialization); if (queue_default_job) { _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL; @@ -1923,10 +1918,7 @@ finish: * getopt() in argv[], and some cleanups in envp[], * but let's hope that doesn't matter.) */ - if (arg_serialization) { - fclose(arg_serialization); - arg_serialization = NULL; - } + arg_serialization = safe_fclose(arg_serialization); if (fds) { fdset_free(fds); @@ -1966,10 +1958,7 @@ finish: log_warning_errno(errno, "Failed to execute /sbin/init, giving up: %m"); } - if (arg_serialization) { - fclose(arg_serialization); - arg_serialization = NULL; - } + arg_serialization = safe_fclose(arg_serialization); if (fds) { fdset_free(fds); diff --git a/src/core/mount.c b/src/core/mount.c index 83746ca412..1f02aa5566 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1540,10 +1540,7 @@ static void mount_shutdown(Manager *m) { m->mount_event_source = sd_event_source_unref(m->mount_event_source); m->mount_utab_event_source = sd_event_source_unref(m->mount_utab_event_source); - if (m->proc_self_mountinfo) { - fclose(m->proc_self_mountinfo); - m->proc_self_mountinfo = NULL; - } + m->proc_self_mountinfo = safe_fclose(m->proc_self_mountinfo); m->utab_inotify_fd = safe_close(m->utab_inotify_fd); } diff --git a/src/core/swap.c b/src/core/swap.c index a26bc58cfc..311ce7ee04 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -1252,10 +1252,7 @@ static void swap_shutdown(Manager *m) { m->swap_event_source = sd_event_source_unref(m->swap_event_source); - if (m->proc_swaps) { - fclose(m->proc_swaps); - m->proc_swaps = NULL; - } + m->proc_swaps = safe_fclose(m->proc_swaps); hashmap_free(m->swaps_by_devnode); m->swaps_by_devnode = NULL; diff --git a/src/dbus1-generator/dbus1-generator.c b/src/dbus1-generator/dbus1-generator.c index 4980fccc31..7bbec5467e 100644 --- a/src/dbus1-generator/dbus1-generator.c +++ b/src/dbus1-generator/dbus1-generator.c @@ -103,8 +103,7 @@ static int create_dbus_files( if (r < 0) return log_error_errno(r, "Failed to write %s: %m", a); - fclose(f); - f = NULL; + f = safe_fclose(f); service = s; } diff --git a/src/journal-remote/journal-gatewayd.c b/src/journal-remote/journal-gatewayd.c index bb762c8832..4e5572db0b 100644 --- a/src/journal-remote/journal-gatewayd.c +++ b/src/journal-remote/journal-gatewayd.c @@ -105,8 +105,7 @@ static void request_meta_free( sd_journal_close(m->journal); - if (m->tmp) - fclose(m->tmp); + safe_fclose(m->tmp); free(m->cursor); free(m); diff --git a/src/journal/coredump.c b/src/journal/coredump.c index 62483a2a05..7d94b145c9 100644 --- a/src/journal/coredump.c +++ b/src/journal/coredump.c @@ -512,8 +512,7 @@ static int compose_open_fds(pid_t pid, char **open_fds) { } errno = 0; - fclose(stream); - stream = NULL; + stream = safe_fclose(stream); if (errno != 0) return -errno; diff --git a/src/journal/stacktrace.c b/src/journal/stacktrace.c index 706c08eac7..98a54ff269 100644 --- a/src/journal/stacktrace.c +++ b/src/journal/stacktrace.c @@ -177,8 +177,7 @@ int coredump_make_stack_trace(int fd, const char *executable, char **ret) { goto finish; } - fclose(c.f); - c.f = NULL; + c.f = safe_fclose(c.f); *ret = buf; buf = NULL; @@ -192,8 +191,7 @@ finish: if (c.elf) elf_end(c.elf); - if (c.f) - fclose(c.f); + safe_fclose(c.f); free(buf); diff --git a/src/libsystemd/sd-bus/bus-introspect.c b/src/libsystemd/sd-bus/bus-introspect.c index 3107d00092..3149a56397 100644 --- a/src/libsystemd/sd-bus/bus-introspect.c +++ b/src/libsystemd/sd-bus/bus-introspect.c @@ -204,8 +204,7 @@ int introspect_finish(struct introspect *i, sd_bus *bus, sd_bus_message *m, sd_b void introspect_free(struct introspect *i) { assert(i); - if (i->f) - fclose(i->f); + safe_fclose(i->f); free(i->introspection); zero(*i); diff --git a/src/libsystemd/sd-hwdb/sd-hwdb.c b/src/libsystemd/sd-hwdb/sd-hwdb.c index 06c98314e8..f0316be659 100644 --- a/src/libsystemd/sd-hwdb/sd-hwdb.c +++ b/src/libsystemd/sd-hwdb/sd-hwdb.c @@ -344,8 +344,7 @@ _public_ sd_hwdb *sd_hwdb_unref(sd_hwdb *hwdb) { if (hwdb && REFCNT_DEC(hwdb->n_ref) == 0) { if (hwdb->map) munmap((void *)hwdb->map, hwdb->st.st_size); - if (hwdb->f) - fclose(hwdb->f); + safe_fclose(hwdb->f); free(hwdb->modalias); ordered_hashmap_free(hwdb->properties); free(hwdb); diff --git a/src/shared/pager.c b/src/shared/pager.c index 55fd5cb79e..479a9d5e8d 100644 --- a/src/shared/pager.c +++ b/src/shared/pager.c @@ -151,13 +151,10 @@ void pager_close(void) { return; /* Inform pager that we are done */ - fclose(stdout); - stdout = NULL; + stdout = safe_fclose(stdout); + stderr = safe_fclose(stderr); - fclose(stderr); - stderr = NULL; - - kill(pager_pid, SIGCONT); + (void) kill(pager_pid, SIGCONT); (void) wait_for_terminate(pager_pid, NULL); pager_pid = 0; }