From 019b3a5d7530c51aa8f7f1e5f5cb5eb81113d4db Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Fri, 3 Aug 2018 18:53:09 +0200
Subject: [PATCH] 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
(cherry picked from commit 8c29a4570993105fecc12288596d2ee77c7f82b8)
Related: #1642460
---
src/login/logind-core.c | 14 ++++++----
src/login/logind-seat.c | 38 +++++++++++++++----------
src/login/logind-seat.h | 6 ++--
src/login/logind-session.c | 57 +++++++++++++++++++++-----------------
src/login/logind-session.h | 7 +++--
src/login/logind-user.c | 21 +++++++-------
6 files changed, 83 insertions(+), 60 deletions(-)
diff --git a/src/login/logind-core.c b/src/login/logind-core.c
index cff5536ac0..f598bbaa1c 100644
--- a/src/login/logind-core.c
+++ b/src/login/logind-core.c
@@ -88,15 +88,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)
@@ -107,15 +108,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 63253db5bf..f68fc0ceaa 100644
--- 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 70878bbe52..51cd468e26 100644
--- 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 69d5a10319..5621d59a41 100644
--- 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 29ca399daf..572f2545c1 100644
--- 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 56b8066f12..60ccd62abb 100644
--- 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) {