Merge pull request #1218 from poettering/safe-fclose

util: introduce safe_fclose() and port everything over to it

14 files changed, 79 insertions, 49 deletions

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 8313675477..5fa44b5cf3 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 b4047b0d14..83ad90c222 100644
--- a/src/bootchart/bootchart.c
+++ b/src/bootchart/bootchart.c
@@ -457,10 +457,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 48b5057a85..be95dc68b2 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;
 }
@@ -1679,10 +1677,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;
 }