logind: rework Seat/Session/User object allocation and freeing a bit

Let's update things a bit to follow current practices:

- User structure initialization rather than zero-initialized allocation

- Always propagate proper errors from allocation functions

- Use _cleanup_ for freeing objects when allocation fails half-way

- Make destructors return NULL

diff --git a/src/login/logind-core.c b/src/login/logind-core.c
index 87506088a1c4a52b16a1fc60f936a349d438a307..3fe2838fc21ad1ef08f93a633352230ecc43b7f5 100644 (file)
--- a/src/login/logind-core.c
+++ b/src/login/logind-core.c
@@ -90,15 +90,16 @@ int manager_add_device(Manager *m, const char *sysfs, bool master, Device **_dev
 
 int manager_add_seat(Manager *m, const char *id, Seat **_seat) {
         Seat *s;
+        int r;
 
         assert(m);
         assert(id);
 
         s = hashmap_get(m->seats, id);
         if (!s) {
-                s = seat_new(m, id);
-                if (!s)
-                        return -ENOMEM;
+                r = seat_new(&s, m, id);
+                if (r < 0)
+                        return r;
         }
 
         if (_seat)
@@ -109,15 +110,16 @@ int manager_add_seat(Manager *m, const char *id, Seat **_seat) {
 
 int manager_add_session(Manager *m, const char *id, Session **_session) {
         Session *s;
+        int r;
 
         assert(m);
         assert(id);
 
         s = hashmap_get(m->sessions, id);
         if (!s) {
-                s = session_new(m, id);
-                if (!s)
-                        return -ENOMEM;
+                r = session_new(&s, m, id);
+                if (r < 0)
+                        return r;
         }
 
         if (_session)

diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index f24fe96841eae0720a7203e3624e8eeacece5de3..398c74d712830d6713891e39363c3fda152ae333 100644 (file)
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -21,33 +21,42 @@
 #include "terminal-util.h"
 #include "util.h"
 
-Seat *seat_new(Manager *m, const char *id) {
-        Seat *s;
+int seat_new(Seat** ret, Manager *m, const char *id) {
+        _cleanup_(seat_freep) Seat *s = NULL;
+        int r;
 
+        assert(ret);
         assert(m);
         assert(id);
 
-        s = new0(Seat, 1);
+        if (!seat_name_is_valid(id))
+                return -EINVAL;
+
+        s = new(Seat, 1);
         if (!s)
-                return NULL;
+                return -ENOMEM;
+
+        *s = (Seat) {
+                .manager = m,
+        };
 
         s->state_file = strappend("/run/systemd/seats/", id);
         if (!s->state_file)
-                return mfree(s);
+                return -ENOMEM;
 
         s->id = basename(s->state_file);
-        s->manager = m;
 
-        if (hashmap_put(m->seats, s->id, s) < 0) {
-                free(s->state_file);
-                return mfree(s);
-        }
+        r = hashmap_put(m->seats, s->id, s);
+        if (r < 0)
+                return r;
 
-        return s;
+        *ret = TAKE_PTR(s);
+        return 0;
 }
 
-void seat_free(Seat *s) {
-        assert(s);
+Seat* seat_free(Seat *s) {
+        if (!s)
+                return NULL;
 
         if (s->in_gc_queue)
                 LIST_REMOVE(gc_queue, s->manager->seat_gc_queue, s);
@@ -64,7 +73,8 @@ void seat_free(Seat *s) {
 
         free(s->positions);
         free(s->state_file);
-        free(s);
+
+        return mfree(s);
 }
 
 int seat_save(Seat *s) {

diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h
index 70878bbe5269c1cf644bfa8be3b9f33cd3c8c3bf..51cd468e26f649bce1d84be657bd13678717c7fd 100644 (file)
--- a/src/login/logind-seat.h
+++ b/src/login/logind-seat.h
@@ -27,8 +27,10 @@ struct Seat {
         LIST_FIELDS(Seat, gc_queue);
 };
 
-Seat *seat_new(Manager *m, const char *id);
-void seat_free(Seat *s);
+int seat_new(Seat **ret, Manager *m, const char *id);
+Seat* seat_free(Seat *s);
+
+DEFINE_TRIVIAL_CLEANUP_FUNC(Seat *, seat_free);
 
 int seat_save(Seat *s);
 int seat_load(Seat *s);

diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 226cc49b0108c7a061baf70e933974d6f43e236a..f59e62837eaaf73c1cf6282daa89cc5d3d171546 100644 (file)
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -24,57 +24,61 @@
 #include "mkdir.h"
 #include "parse-util.h"
 #include "path-util.h"
+#include "process-util.h"
 #include "string-table.h"
 #include "terminal-util.h"
 #include "user-util.h"
 #include "util.h"
-#include "process-util.h"
 
 #define RELEASE_USEC (20*USEC_PER_SEC)
 
 static void session_remove_fifo(Session *s);
 
-Session* session_new(Manager *m, const char *id) {
-        Session *s;
+int session_new(Session **ret, Manager *m, const char *id) {
+        _cleanup_(session_freep) Session *s = NULL;
+        int r;
 
+        assert(ret);
         assert(m);
         assert(id);
-        assert(session_id_valid(id));
 
-        s = new0(Session, 1);
+        if (!session_id_valid(id))
+                return -EINVAL;
+
+        s = new(Session, 1);
         if (!s)
-                return NULL;
+                return -ENOMEM;
+
+        *s = (Session) {
+                .manager = m,
+                .fifo_fd = -1,
+                .vtfd = -1,
+                .audit_id = AUDIT_SESSION_INVALID,
+        };
 
         s->state_file = strappend("/run/systemd/sessions/", id);
         if (!s->state_file)
-                return mfree(s);
-
-        s->devices = hashmap_new(&devt_hash_ops);
-        if (!s->devices) {
-                free(s->state_file);
-                return mfree(s);
-        }
+                return -ENOMEM;
 
         s->id = basename(s->state_file);
 
-        if (hashmap_put(m->sessions, s->id, s) < 0) {
-                hashmap_free(s->devices);
-                free(s->state_file);
-                return mfree(s);
-        }
+        s->devices = hashmap_new(&devt_hash_ops);
+        if (!s->devices)
+                return -ENOMEM;
 
-        s->manager = m;
-        s->fifo_fd = -1;
-        s->vtfd = -1;
-        s->audit_id = AUDIT_SESSION_INVALID;
+        r = hashmap_put(m->sessions, s->id, s);
+        if (r < 0)
+                return r;
 
-        return s;
+        *ret = TAKE_PTR(s);
+        return 0;
 }
 
-void session_free(Session *s) {
+Session* session_free(Session *s) {
         SessionDevice *sd;
 
-        assert(s);
+        if (!s)
+                return NULL;
 
         if (s->in_gc_queue)
                 LIST_REMOVE(gc_queue, s->manager->session_gc_queue, s);
@@ -126,7 +130,8 @@ void session_free(Session *s) {
         hashmap_remove(s->manager->sessions, s->id);
 
         free(s->state_file);
-        free(s);
+
+        return mfree(s);
 }
 
 void session_set_user(Session *s, User *u) {

diff --git a/src/login/logind-session.h b/src/login/logind-session.h
index 29ca399dafac365c3eb110a303db7612edf970c6..572f2545c138105da8b7d24b9784997ad6570435 100644 (file)
--- a/src/login/logind-session.h
+++ b/src/login/logind-session.h
@@ -109,8 +109,11 @@ struct Session {
         LIST_FIELDS(Session, gc_queue);
 };
 
-Session *session_new(Manager *m, const char *id);
-void session_free(Session *s);
+int session_new(Session **ret, Manager *m, const char *id);
+Session* session_free(Session *s);
+
+DEFINE_TRIVIAL_CLEANUP_FUNC(Session *, session_free);
+
 void session_set_user(Session *s, User *u);
 bool session_may_gc(Session *s, bool drop_not_started);
 void session_add_to_gc_queue(Session *s);

diff --git a/src/login/logind-user.c b/src/login/logind-user.c
index f2664c323ea86367717bfb8a6035b7a3a6af16a8..ad6349944c48e75be90ff69543e9a0ccc8ac6d31 100644 (file)
--- a/src/login/logind-user.c
+++ b/src/login/logind-user.c
@@ -30,23 +30,24 @@
 #include "user-util.h"
 #include "util.h"
 
-int user_new(User **out, Manager *m, uid_t uid, gid_t gid, const char *name) {
+int user_new(User **ret, Manager *m, uid_t uid, gid_t gid, const char *name) {
         _cleanup_(user_freep) User *u = NULL;
         char lu[DECIMAL_STR_MAX(uid_t) + 1];
         int r;
 
-        assert(out);
+        assert(ret);
         assert(m);
         assert(name);
 
-        u = new0(User, 1);
+        u = new(User, 1);
         if (!u)
                 return -ENOMEM;
 
-        u->manager = m;
-        u->uid = uid;
-        u->gid = gid;
-        xsprintf(lu, UID_FMT, uid);
+        *u = (User) {
+                .manager = m,
+                .uid = uid,
+                .gid = gid,
+        };
 
         u->name = strdup(name);
         if (!u->name)
@@ -58,6 +59,7 @@ int user_new(User **out, Manager *m, uid_t uid, gid_t gid, const char *name) {
         if (asprintf(&u->runtime_path, "/run/user/"UID_FMT, uid) < 0)
                 return -ENOMEM;
 
+        xsprintf(lu, UID_FMT, uid);
         r = slice_build_subslice(SPECIAL_USER_SLICE, lu, &u->slice);
         if (r < 0)
                 return r;
@@ -78,8 +80,7 @@ int user_new(User **out, Manager *m, uid_t uid, gid_t gid, const char *name) {
         if (r < 0)
                 return r;
 
-        *out = TAKE_PTR(u);
-
+        *ret = TAKE_PTR(u);
         return 0;
 }
 
@@ -272,7 +273,7 @@ int user_save(User *u) {
         if (!u->started)
                 return 0;
 
-        return user_save_internal (u);
+        return user_save_internal(u);
 }
 
 int user_load(User *u) {