From 157f50577fbee094eb8ca18f3f0af4e82af8558f Mon Sep 17 00:00:00 2001
From: David Herrmann <dh.herrmann@gmail.com>
Date: Tue, 29 Sep 2015 11:10:01 +0200
Subject: [PATCH] login: make user_new() and user_free() follow coding-style

Few changes to user_new() and user_free():
 - Use _cleanup_(user_freep) in constructor
 - return 'int' from user_new()
 - make user_free() deal with partially initialized objects
 - keep reverse-order in user_free() compared to user_new()
 - make user_free() return NULL
 - make user_free() accept NULL as no-op
---
 src/login/logind-core.c |  7 +++--
 src/login/logind-user.c | 83 +++++++++++++++++++++++--------------------------
 src/login/logind-user.h |  7 +++--
 3 files changed, 48 insertions(+), 49 deletions(-)

diff --git a/src/login/logind-core.c b/src/login/logind-core.c
index b3f30c8..38c426c 100644
--- a/src/login/logind-core.c
+++ b/src/login/logind-core.c
@@ -98,15 +98,16 @@ int manager_add_session(Manager *m, const char *id, Session **_session) {
 
 int manager_add_user(Manager *m, uid_t uid, gid_t gid, const char *name, User **_user) {
         User *u;
+        int r;
 
         assert(m);
         assert(name);
 
         u = hashmap_get(m->users, UID_TO_PTR(uid));
         if (!u) {
-                u = user_new(m, uid, gid, name);
-                if (!u)
-                        return -ENOMEM;
+                r = user_new(&u, m, uid, gid, name);
+                if (r < 0)
+                        return r;
         }
 
         if (_user)
diff --git a/src/login/logind-user.c b/src/login/logind-user.c
index d985d19..292a583 100644
--- a/src/login/logind-user.c
+++ b/src/login/logind-user.c
@@ -51,60 +51,54 @@
 #include "user-util.h"
 #include "util.h"
 
-User* user_new(Manager *m, uid_t uid, gid_t gid, const char *name) {
+int user_new(User **out, 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];
-        User *u;
         int r;
 
+        assert(out);
         assert(m);
         assert(name);
 
         u = new0(User, 1);
         if (!u)
-                return NULL;
+                return -ENOMEM;
+
+        u->manager = m;
+        u->uid = uid;
+        u->gid = gid;
 
         u->name = strdup(name);
         if (!u->name)
-                goto fail;
+                return -ENOMEM;
 
         if (asprintf(&u->state_file, "/run/systemd/users/"UID_FMT, uid) < 0)
-                goto fail;
+                return -ENOMEM;
 
         if (asprintf(&u->runtime_path, "/run/user/"UID_FMT, uid) < 0)
-                goto fail;
+                return -ENOMEM;
 
-        sprintf(lu, UID_FMT, uid);
+        xsprintf(lu, UID_FMT, uid);
         r = slice_build_subslice(SPECIAL_USER_SLICE, lu, &u->slice);
         if (r < 0)
-                goto fail;
-
-        if (hashmap_put(m->users, UID_TO_PTR(uid), u) < 0)
-                goto fail;
-
-        if (hashmap_put(m->user_units, u->slice, u) < 0)
-                goto fail;
+                return r;
 
-        u->manager = m;
-        u->uid = uid;
-        u->gid = gid;
+        r = hashmap_put(m->users, UID_TO_PTR(uid), u);
+        if (r < 0)
+                return r;
 
-        return u;
+        r = hashmap_put(m->user_units, u->slice, u);
+        if (r < 0)
+                return r;
 
-fail:
-        if (u->slice)
-                hashmap_remove(m->user_units, u->slice);
-        hashmap_remove(m->users, UID_TO_PTR(uid));
-        free(u->slice);
-        free(u->runtime_path);
-        free(u->state_file);
-        free(u->name);
-        free(u);
-
-        return NULL;
+        *out = u;
+        u = NULL;
+        return 0;
 }
 
-void user_free(User *u) {
-        assert(u);
+User *user_free(User *u) {
+        if (!u)
+                return NULL;
 
         if (u->in_gc_queue)
                 LIST_REMOVE(gc_queue, u->manager->user_gc_queue, u);
@@ -112,23 +106,24 @@ void user_free(User *u) {
         while (u->sessions)
                 session_free(u->sessions);
 
-        if (u->service) {
-                hashmap_remove(u->manager->user_units, u->service);
-                free(u->service);
-        }
+        if (u->service)
+                hashmap_remove_value(u->manager->user_units, u->service, u);
 
-        hashmap_remove(u->manager->user_units, u->slice);
-        hashmap_remove(u->manager->users, UID_TO_PTR(u->uid));
+        if (u->slice)
+                hashmap_remove_value(u->manager->user_units, u->slice, u);
 
-        free(u->slice_job);
-        free(u->service_job);
+        hashmap_remove_value(u->manager->users, UID_TO_PTR(u->uid), u);
+
+        u->slice_job = mfree(u->slice_job);
+        u->service_job = mfree(u->service_job);
 
-        free(u->slice);
-        free(u->runtime_path);
+        u->service = mfree(u->service);
+        u->slice = mfree(u->slice);
+        u->runtime_path = mfree(u->runtime_path);
+        u->state_file = mfree(u->state_file);
+        u->name = mfree(u->name);
 
-        free(u->name);
-        free(u->state_file);
-        free(u);
+        return mfree(u);
 }
 
 static int user_save_internal(User *u) {
diff --git a/src/login/logind-user.h b/src/login/logind-user.h
index 7222478..11d28d2 100644
--- a/src/login/logind-user.h
+++ b/src/login/logind-user.h
@@ -65,8 +65,11 @@ struct User {
         LIST_FIELDS(User, gc_queue);
 };
 
-User* user_new(Manager *m, uid_t uid, gid_t gid, const char *name);
-void user_free(User *u);
+int user_new(User **out, Manager *m, uid_t uid, gid_t gid, const char *name);
+User *user_free(User *u);
+
+DEFINE_TRIVIAL_CLEANUP_FUNC(User *, user_free);
+
 bool user_check_gc(User *u, bool drop_not_started);
 void user_add_to_gc_queue(User *u);
 int user_start(User *u);
-- 
2.7.4