From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 23 Jul 2019 08:27:19 +0000 (+0200)
Subject: logind: rework allocation/freeing of inhibitors
X-Git-Tag: v243-rc1~56^2~4
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=81280b2a6fbad079b5d72f48160bf1d492cc8e57;p=thirdparty%2Fsystemd.git

logind: rework allocation/freeing of inhibitors

Let's follow our modern style (i.e. return proper errors, use structure
initialization and _cleanup_).

Most importantly: remove state file and FIFO removal from
inhibitor_free() and let's move it to inhibitor_stop().

This makes sure that state files/FIFOs are not removed when the we
terminate logind, i.e. that they can survive logind restarts.

Fixes: #11825
---

diff --git a/src/login/logind-core.c b/src/login/logind-core.c
index f631ba7219c..1d21e90a2ea 100644
--- a/src/login/logind-core.c
+++ b/src/login/logind-core.c
@@ -197,26 +197,22 @@ int manager_add_user_by_uid(Manager *m, uid_t uid, User **_user) {
         return manager_add_user(m, uid, p->pw_gid, p->pw_name, p->pw_dir, _user);
 }
 
-int manager_add_inhibitor(Manager *m, const char* id, Inhibitor **_inhibitor) {
+int manager_add_inhibitor(Manager *m, const char* id, Inhibitor **ret) {
         Inhibitor *i;
+        int r;
 
         assert(m);
         assert(id);
 
         i = hashmap_get(m->inhibitors, id);
-        if (i) {
-                if (_inhibitor)
-                        *_inhibitor = i;
-
-                return 0;
+        if (!i) {
+                r = inhibitor_new(&i, m, id);
+                if (r < 0)
+                        return r;
         }
 
-        i = inhibitor_new(m, id);
-        if (!i)
-                return -ENOMEM;
-
-        if (_inhibitor)
-                *_inhibitor = i;
+        if (ret)
+                *ret = i;
 
         return 0;
 }
diff --git a/src/login/logind-inhibit.c b/src/login/logind-inhibit.c
index 8716a9f091e..11cbb4534c5 100644
--- a/src/login/logind-inhibit.c
+++ b/src/login/logind-inhibit.c
@@ -24,48 +24,59 @@
 #include "user-util.h"
 #include "util.h"
 
-Inhibitor* inhibitor_new(Manager *m, const char* id) {
-        Inhibitor *i;
+int inhibitor_new(Inhibitor **ret, Manager *m, const char* id) {
+        _cleanup_(inhibitor_freep) Inhibitor *i = NULL;
+        int r;
 
+        assert(ret);
         assert(m);
+        assert(id);
 
-        i = new0(Inhibitor, 1);
+        i = new(Inhibitor, 1);
         if (!i)
-                return NULL;
+                return -ENOMEM;
+
+        *i = (Inhibitor) {
+                .manager = m,
+                .what = _INHIBIT_WHAT_INVALID,
+                .mode = _INHIBIT_MODE_INVALID,
+                .uid = UID_INVALID,
+                .fifo_fd = -1,
+        };
 
         i->state_file = path_join("/run/systemd/inhibit", id);
         if (!i->state_file)
-                return mfree(i);
+                return -ENOMEM;
 
         i->id = basename(i->state_file);
 
-        if (hashmap_put(m->inhibitors, i->id, i) < 0) {
-                free(i->state_file);
-                return mfree(i);
-        }
-
-        i->manager = m;
-        i->fifo_fd = -1;
+        r = hashmap_put(m->inhibitors, i->id, i);
+        if (r < 0)
+                return r;
 
-        return i;
+        *ret = TAKE_PTR(i);
+        return 0;
 }
 
-void inhibitor_free(Inhibitor *i) {
-        assert(i);
-
-        hashmap_remove(i->manager->inhibitors, i->id);
+Inhibitor* inhibitor_free(Inhibitor *i) {
 
-        inhibitor_remove_fifo(i);
+        if (!i)
+                return NULL;
 
         free(i->who);
         free(i->why);
 
-        if (i->state_file) {
-                (void) unlink(i->state_file);
-                free(i->state_file);
-        }
+        sd_event_source_unref(i->event_source);
+        safe_close(i->fifo_fd);
 
-        free(i);
+        /* Note that we don't remove neither the state file nor the fifo path here, since we want both to
+         * survive daemon restarts */
+        free(i->state_file);
+        free(i->fifo_path);
+
+        hashmap_remove(i->manager->inhibitors, i->id);
+
+        return mfree(i);
 }
 
 int inhibitor_save(Inhibitor *i) {
@@ -184,6 +195,8 @@ int inhibitor_stop(Inhibitor *i) {
                           i->pid, i->uid,
                           inhibit_mode_to_string(i->mode));
 
+        inhibitor_remove_fifo(i);
+
         if (i->state_file)
                 (void) unlink(i->state_file);
 
diff --git a/src/login/logind-inhibit.h b/src/login/logind-inhibit.h
index 650587106da..a5cec63ecd3 100644
--- a/src/login/logind-inhibit.h
+++ b/src/login/logind-inhibit.h
@@ -48,8 +48,10 @@ struct Inhibitor {
         int fifo_fd;
 };
 
-Inhibitor* inhibitor_new(Manager *m, const char *id);
-void inhibitor_free(Inhibitor *i);
+int inhibitor_new(Inhibitor **ret, Manager *m, const char* id);
+Inhibitor* inhibitor_free(Inhibitor *i);
+
+DEFINE_TRIVIAL_CLEANUP_FUNC(Inhibitor*, inhibitor_free);
 
 int inhibitor_save(Inhibitor *i);
 int inhibitor_load(Inhibitor *i);