From: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Date: Fri, 11 Oct 2019 10:48:33 +0000 (+0200)
Subject: core: adjust load functions for other unit types to be more like service
X-Git-Tag: v244~235^2~2
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=75193d4128c08291a17b0d7be11722d071650425;p=platform%2Fupstream%2Fsystemd.git

core: adjust load functions for other unit types to be more like service

No functional change, just adjusting code to follow the same pattern
everywhere. In particular, never call _verify() on an already loaded unit,
but return early from the caller instead. This makes the code a bit easier
to follow.
---

diff --git a/src/core/automount.c b/src/core/automount.c
index c15520c..0ecacc1 100644
--- a/src/core/automount.c
+++ b/src/core/automount.c
@@ -164,9 +164,7 @@ static int automount_verify(Automount *a) {
         int r;
 
         assert(a);
-
-        if (UNIT(a)->load_state != UNIT_LOADED)
-                return 0;
+        assert(UNIT(a)->load_state == UNIT_LOADED);
 
         if (path_equal(a->where, "/")) {
                 log_unit_error(UNIT(a), "Cannot have an automount unit for the root directory. Refusing.");
@@ -201,6 +199,24 @@ static int automount_set_where(Automount *a) {
         return 1;
 }
 
+static int automount_add_extras(Automount *a) {
+        int r;
+
+        r = automount_set_where(a);
+        if (r < 0)
+                return r;
+
+        r = automount_add_trigger_dependencies(a);
+        if (r < 0)
+                return r;
+
+        r = automount_add_mount_dependencies(a);
+        if (r < 0)
+                return r;
+
+        return automount_add_default_dependencies(a);
+}
+
 static int automount_load(Unit *u) {
         Automount *a = AUTOMOUNT(u);
         int r;
@@ -213,23 +229,12 @@ static int automount_load(Unit *u) {
         if (r < 0)
                 return r;
 
-        if (u->load_state == UNIT_LOADED) {
-                r = automount_set_where(a);
-                if (r < 0)
-                        return r;
-
-                r = automount_add_trigger_dependencies(a);
-                if (r < 0)
-                        return r;
-
-                r = automount_add_mount_dependencies(a);
-                if (r < 0)
-                        return r;
+        if (u->load_state != UNIT_LOADED)
+                return 0;
 
-                r = automount_add_default_dependencies(a);
-                if (r < 0)
-                        return r;
-        }
+        r = automount_add_extras(a);
+        if (r < 0)
+                return r;
 
         return automount_verify(a);
 }
diff --git a/src/core/mount.c b/src/core/mount.c
index bd851ac..7697a74 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -513,9 +513,7 @@ static int mount_verify(Mount *m) {
         int r;
 
         assert(m);
-
-        if (UNIT(m)->load_state != UNIT_LOADED)
-                return 0;
+        assert(UNIT(m)->load_state == UNIT_LOADED);
 
         if (!m->from_fragment && !m->from_proc_self_mountinfo && !UNIT(m)->perpetual)
                 return -ENOENT;
@@ -606,11 +604,11 @@ static int mount_add_extras(Mount *m) {
         return 0;
 }
 
-static int mount_load_root_mount(Unit *u) {
+static void mount_load_root_mount(Unit *u) {
         assert(u);
 
         if (!unit_has_name(u, SPECIAL_ROOT_MOUNT))
-                return 0;
+                return;
 
         u->perpetual = true;
         u->default_dependencies = false;
@@ -621,37 +619,33 @@ static int mount_load_root_mount(Unit *u) {
 
         if (!u->description)
                 u->description = strdup("Root Mount");
-
-        return 1;
 }
 
 static int mount_load(Unit *u) {
         Mount *m = MOUNT(u);
-        int r, q, w;
+        int r, q = 0;
 
         assert(u);
         assert(u->load_state == UNIT_STUB);
 
-        r = mount_load_root_mount(u);
+        mount_load_root_mount(u);
 
         bool fragment_optional = m->from_proc_self_mountinfo || u->perpetual;
-        q = unit_load_fragment_and_dropin(u, !fragment_optional);
+        r = unit_load_fragment_and_dropin(u, !fragment_optional);
 
         /* Add in some extras. Note we do this in all cases (even if we failed to load the unit) when announced by the
          * kernel, because we need some things to be set up no matter what when the kernel establishes a mount and thus
          * we need to update the state in our unit to track it. After all, consider that we don't allow changing the
          * 'slice' field for a unit once it is active. */
         if (u->load_state == UNIT_LOADED || m->from_proc_self_mountinfo || u->perpetual)
-                w = mount_add_extras(m);
-        else
-                w = 0;
+                q = mount_add_extras(m);
 
         if (r < 0)
                 return r;
         if (q < 0)
                 return q;
-        if (w < 0)
-                return w;
+        if (u->load_state != UNIT_LOADED)
+                return 0;
 
         return mount_verify(m);
 }
diff --git a/src/core/path.c b/src/core/path.c
index e7071cc..dff551f 100644
--- a/src/core/path.c
+++ b/src/core/path.c
@@ -284,9 +284,7 @@ static int path_add_mount_dependencies(Path *p) {
 
 static int path_verify(Path *p) {
         assert(p);
-
-        if (UNIT(p)->load_state != UNIT_LOADED)
-                return 0;
+        assert(UNIT(p)->load_state == UNIT_LOADED);
 
         if (!p->specs) {
                 log_unit_error(UNIT(p), "Path unit lacks path setting. Refusing.");
@@ -333,6 +331,20 @@ static int path_add_trigger_dependencies(Path *p) {
         return unit_add_two_dependencies(UNIT(p), UNIT_BEFORE, UNIT_TRIGGERS, x, true, UNIT_DEPENDENCY_IMPLICIT);
 }
 
+static int path_add_extras(Path *p) {
+        int r;
+
+        r = path_add_trigger_dependencies(p);
+        if (r < 0)
+                return r;
+
+        r = path_add_mount_dependencies(p);
+        if (r < 0)
+                return r;
+
+        return path_add_default_dependencies(p);
+}
+
 static int path_load(Unit *u) {
         Path *p = PATH(u);
         int r;
@@ -344,20 +356,12 @@ static int path_load(Unit *u) {
         if (r < 0)
                 return r;
 
-        if (u->load_state == UNIT_LOADED) {
-
-                r = path_add_trigger_dependencies(p);
-                if (r < 0)
-                        return r;
-
-                r = path_add_mount_dependencies(p);
-                if (r < 0)
-                        return r;
+        if (u->load_state != UNIT_LOADED)
+                return 0;
 
-                r = path_add_default_dependencies(p);
-                if (r < 0)
-                        return r;
-        }
+        r = path_add_extras(p);
+        if (r < 0)
+                return r;
 
         return path_verify(p);
 }
diff --git a/src/core/scope.c b/src/core/scope.c
index e03a1c7..094c897 100644
--- a/src/core/scope.c
+++ b/src/core/scope.c
@@ -125,9 +125,7 @@ static int scope_add_default_dependencies(Scope *s) {
 
 static int scope_verify(Scope *s) {
         assert(s);
-
-        if (UNIT(s)->load_state != UNIT_LOADED)
-                return 0;
+        assert(UNIT(s)->load_state == UNIT_LOADED);
 
         if (set_isempty(UNIT(s)->pids) &&
             !MANAGER_IS_RELOADING(UNIT(s)->manager) &&
@@ -162,6 +160,20 @@ static int scope_load_init_scope(Unit *u) {
         return 1;
 }
 
+static int scope_add_extras(Scope *s) {
+        int r;
+
+        r = unit_patch_contexts(UNIT(s));
+        if (r < 0)
+                return r;
+
+        r = unit_set_default_slice(UNIT(s));
+        if (r < 0)
+                return r;
+
+        return scope_add_default_dependencies(s);
+}
+
 static int scope_load(Unit *u) {
         Scope *s = SCOPE(u);
         int r;
@@ -181,19 +193,12 @@ static int scope_load(Unit *u) {
         if (r < 0)
                 return r;
 
-        if (u->load_state == UNIT_LOADED) {
-                r = unit_patch_contexts(u);
-                if (r < 0)
-                        return r;
-
-                r = unit_set_default_slice(u);
-                if (r < 0)
-                        return r;
+        if (u->load_state != UNIT_LOADED)
+                return 0;
 
-                r = scope_add_default_dependencies(s);
-                if (r < 0)
-                        return r;
-        }
+        r = scope_add_extras(s);
+        if (r < 0)
+                return r;
 
         return scope_verify(s);
 }
diff --git a/src/core/slice.c b/src/core/slice.c
index 9ef1202..d97a262 100644
--- a/src/core/slice.c
+++ b/src/core/slice.c
@@ -91,9 +91,7 @@ static int slice_verify(Slice *s) {
         int r;
 
         assert(s);
-
-        if (UNIT(s)->load_state != UNIT_LOADED)
-                return 0;
+        assert(UNIT(s)->load_state == UNIT_LOADED);
 
         if (!slice_name_is_valid(UNIT(s)->id)) {
                 log_unit_error(UNIT(s), "Slice name %s is not valid. Refusing.", UNIT(s)->id);
@@ -174,21 +172,21 @@ static int slice_load(Unit *u) {
         if (r < 0)
                 return r;
 
-        /* This is a new unit? Then let's add in some extras */
-        if (u->load_state == UNIT_LOADED) {
+        if (u->load_state != UNIT_LOADED)
+                return 0;
 
-                r = unit_patch_contexts(u);
-                if (r < 0)
-                        return r;
+        /* This is a new unit? Then let's add in some extras */
+        r = unit_patch_contexts(u);
+        if (r < 0)
+                return r;
 
-                r = slice_add_parent_slice(s);
-                if (r < 0)
-                        return r;
+        r = slice_add_parent_slice(s);
+        if (r < 0)
+                return r;
 
-                r = slice_add_default_dependencies(s);
-                if (r < 0)
-                        return r;
-        }
+        r = slice_add_default_dependencies(s);
+        if (r < 0)
+                return r;
 
         return slice_verify(s);
 }
diff --git a/src/core/socket.c b/src/core/socket.c
index 3d04fdf..71ab3c6 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -433,9 +433,7 @@ static const char *socket_find_symlink_target(Socket *s) {
 
 static int socket_verify(Socket *s) {
         assert(s);
-
-        if (UNIT(s)->load_state != UNIT_LOADED)
-                return 0;
+        assert(UNIT(s)->load_state == UNIT_LOADED);
 
         if (!s->ports) {
                 log_unit_error(UNIT(s), "Unit has no Listen setting (ListenStream=, ListenDatagram=, ListenFIFO=, ...). Refusing.");
@@ -518,12 +516,13 @@ static int socket_load(Unit *u) {
         if (r < 0)
                 return r;
 
-        if (u->load_state == UNIT_LOADED) {
-                /* This is a new unit? Then let's add in some extras */
-                r = socket_add_extras(s);
-                if (r < 0)
-                        return r;
-        }
+        if (u->load_state != UNIT_LOADED)
+                return 0;
+
+        /* This is a new unit? Then let's add in some extras */
+        r = socket_add_extras(s);
+        if (r < 0)
+                return r;
 
         return socket_verify(s);
 }
diff --git a/src/core/swap.c b/src/core/swap.c
index fcea0fd..726bda2 100644
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -232,8 +232,7 @@ static int swap_verify(Swap *s) {
         _cleanup_free_ char *e = NULL;
         int r;
 
-        if (UNIT(s)->load_state != UNIT_LOADED)
-                return 0;
+        assert(UNIT(s)->load_state == UNIT_LOADED);
 
         r = unit_name_from_path(s->what, ".swap", &e);
         if (r < 0)
@@ -340,7 +339,7 @@ static int swap_add_extras(Swap *s) {
 
 static int swap_load(Unit *u) {
         Swap *s = SWAP(u);
-        int r, q;
+        int r, q = 0;
 
         assert(s);
         assert(u->load_state == UNIT_STUB);
@@ -349,17 +348,17 @@ static int swap_load(Unit *u) {
         bool fragment_optional = s->from_proc_swaps;
         r = unit_load_fragment_and_dropin(u, !fragment_optional);
 
-        /* Add in some extras, and do so either when we successfully loaded something or when /proc/swaps is already
-         * active. */
+        /* Add in some extras, and do so either when we successfully loaded something or when /proc/swaps is
+         * already active. */
         if (u->load_state == UNIT_LOADED || s->from_proc_swaps)
                 q = swap_add_extras(s);
-        else
-                q = 0;
 
         if (r < 0)
                 return r;
         if (q < 0)
                 return q;
+        if (u->load_state != UNIT_LOADED)
+                return 0;
 
         return swap_verify(s);
 }
diff --git a/src/core/target.c b/src/core/target.c
index 5671faa..357ca70 100644
--- a/src/core/target.c
+++ b/src/core/target.c
@@ -84,14 +84,11 @@ static int target_load(Unit *u) {
         if (r < 0)
                 return r;
 
-        /* This is a new unit? Then let's add in some extras */
-        if (u->load_state == UNIT_LOADED) {
-                r = target_add_default_dependencies(t);
-                if (r < 0)
-                        return r;
-        }
+        if (u->load_state != UNIT_LOADED)
+                return 0;
 
-        return 0;
+        /* This is a new unit? Then let's add in some extras */
+        return target_add_default_dependencies(t);
 }
 
 static int target_coldplug(Unit *u) {
diff --git a/src/core/timer.c b/src/core/timer.c
index e4821e1..47c59ab 100644
--- a/src/core/timer.c
+++ b/src/core/timer.c
@@ -73,9 +73,7 @@ static void timer_done(Unit *u) {
 
 static int timer_verify(Timer *t) {
         assert(t);
-
-        if (UNIT(t)->load_state != UNIT_LOADED)
-                return 0;
+        assert(UNIT(t)->load_state == UNIT_LOADED);
 
         if (!t->values && !t->on_clock_change && !t->on_timezone_change) {
                 log_unit_error(UNIT(t), "Timer unit lacks value setting. Refusing.");
@@ -182,20 +180,21 @@ static int timer_load(Unit *u) {
         if (r < 0)
                 return r;
 
-        if (u->load_state == UNIT_LOADED) {
+        if (u->load_state != UNIT_LOADED)
+                return 0;
 
-                r = timer_add_trigger_dependencies(t);
-                if (r < 0)
-                        return r;
+        /* This is a new unit? Then let's add in some extras */
+        r = timer_add_trigger_dependencies(t);
+        if (r < 0)
+                return r;
 
-                r = timer_setup_persistent(t);
-                if (r < 0)
-                        return r;
+        r = timer_setup_persistent(t);
+        if (r < 0)
+                return r;
 
-                r = timer_add_default_dependencies(t);
-                if (r < 0)
-                        return r;
-        }
+        r = timer_add_default_dependencies(t);
+        if (r < 0)
+                return r;
 
         return timer_verify(t);
 }