From: Lennart Poettering <lennart@poettering.net>
Date: Mon, 18 Jan 2010 22:50:13 +0000 (+0100)
Subject: first try at implementing dependency loader
X-Git-Tag: v1~849
X-Git-Url: https://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?p=elogind.git;a=commitdiff_plain;h=87d1515de5ae611e95144def9ef4b2c0c933f6fe;hp=91cdde8a7a08c6797995cc67f4b55ac43780cdd8

first try at implementing dependency loader
---

diff --git a/job.c b/job.c
index 5cd8f73a9..689908aeb 100644
--- a/job.c
+++ b/job.c
@@ -47,13 +47,16 @@ void job_free(Job *j) {
         /* Detach from next 'bigger' objects */
 
         if (j->linked) {
-                if (j->name && j->name->meta.job == j)
-                        j->name->meta.job = NULL;
+                assert(j->name);
+                assert(j->name->meta.job == j);
+                j->name->meta.job = NULL;
 
                 hashmap_remove(j->manager->jobs, UINT32_TO_PTR(j->id));
         }
 
-        /* Free data and next 'smaller' objects */
+        hashmap_remove(j->manager->jobs_to_add, j->name);
+        set_remove(j->manager->jobs_to_remove, j);
 
+        /* Free data and next 'smaller' objects */
         free(j);
 }
diff --git a/load-fragment.c b/load-fragment.c
index b5b3e4862..4510cc916 100644
--- a/load-fragment.c
+++ b/load-fragment.c
@@ -9,7 +9,7 @@
 #include "conf-parser.h"
 #include "load-fragment.h"
 
-int config_parse_names(
+int config_parse_deps(
                 const char *filename,
                 unsigned line,
                 const char *section,
@@ -54,41 +54,100 @@ int config_parse_names(
         return 0;
 }
 
+int config_parse_names(
+                const char *filename,
+                unsigned line,
+                const char *section,
+                const char *lvalue,
+                const char *rvalue,
+                void *data,
+                void *userdata) {
+
+        Set **set = data;
+        Name *name = userdata;
+        char *w;
+        size_t l;
+        char *state;
+
+        assert(filename);
+        assert(lvalue);
+        assert(rvalue);
+        assert(data);
+
+        FOREACH_WORD(w, &l, rvalue, state) {
+                char *t;
+                int r;
+                Name *other;
+
+                if (!(t = strndup(w, l)))
+                        return -ENOMEM;
+
+                other = manager_get_name(name->meta.manager, t);
+
+                if (other) {
+
+                        if (other != name) {
+
+                                if (other->meta.state != NAME_STUB) {
+                                        free(t);
+                                        return -EEXIST;
+                                }
+
+                                if ((r = name_merge(name, other) < 0)) {
+                                        free(t);
+                                        return r;
+                                }
+                        }
+
+                } else {
+
+                        if (!*set)
+                                if (!(*set = set_new(trivial_hash_func, trivial_compare_func))) {
+                                        free(t);
+                                        return -ENOMEM;
+                                }
+
+                        if ((r = set_put(*set, t)) < 0) {
+                                free(t);
+                                return r;
+                        }
+                }
+
+                free(t);
+        }
+
+        return 0;
+}
+
 int name_load_fragment(Name *n) {
 
         const ConfigItem items[] = {
-                { "Names",         config_parse_strv,   &n->meta.names,          "Meta" },
-                { "Description",   config_parse_string, &n->meta.description,    "Meta" },
-                { "Requires",      config_parse_names,  &n->meta.requires,       "Meta" },
-                { "SoftRequires",  config_parse_names,  &n->meta.soft_requires,  "Meta" },
-                { "Wants",         config_parse_names,  &n->meta.wants,          "Meta" },
-                { "Requisite",     config_parse_names,  &n->meta.requisite,      "Meta" },
-                { "SoftRequisite", config_parse_names,  &n->meta.soft_requisite, "Meta" },
-                { "Conflicts",     config_parse_names,  &n->meta.conflicts,      "Meta" },
-                { "Before",        config_parse_names,  &n->meta.before,         "Meta" },
-                { "After",         config_parse_names,  &n->meta.after,          "Meta" },
+                { "Names",         config_parse_names,  &n->meta.names,                           "Meta" },
+                { "Description",   config_parse_string, &n->meta.description,                     "Meta" },
+                { "Requires",      config_parse_deps,   n->meta.dependencies+NAME_REQUIRES,       "Meta" },
+                { "SoftRequires",  config_parse_deps,   n->meta.dependencies+NAME_SOFT_REQUIRES,  "Meta" },
+                { "Wants",         config_parse_deps,   n->meta.dependencies+NAME_WANTS,          "Meta" },
+                { "Requisite",     config_parse_deps,   n->meta.dependencies+NAME_REQUISITE,      "Meta" },
+                { "SoftRequisite", config_parse_deps,   n->meta.dependencies+NAME_SOFT_REQUISITE, "Meta" },
+                { "Conflicts",     config_parse_deps,   n->meta.dependencies+NAME_CONFLICTS,      "Meta" },
+                { "Before",        config_parse_deps,   n->meta.dependencies+NAME_BEFORE,         "Meta" },
+                { "After",         config_parse_deps,   n->meta.dependencies+NAME_AFTER,          "Meta" },
                 { NULL, NULL, NULL, NULL }
         };
 
-        char **t, **l;
+        char *t;
         int r;
+        void *state;
 
         assert(n);
         assert(n->meta.state == NAME_STUB);
 
-        /* We copy the strv here so that we can iterate through it
-         * while being safe for modification */
-        if (!(l = strv_copy(n->meta.names)))
-                return -ENOMEM;
-
-        STRV_FOREACH(t, n->meta.names)
-                if ((r = config_parse(*t, items, n)) < 0)
+        SET_FOREACH(t, n->meta.names, state)
+                if ((r = config_parse(t, items, n)) < 0)
                         goto fail;
 
-        return 0;
+        r = 0;
 
 fail:
-
-        strv_free(l);
-        return 0;
+        return r;
 }
diff --git a/manager.c b/manager.c
index 97c99e504..0ad60b2b0 100644
--- a/manager.c
+++ b/manager.c
@@ -2,6 +2,7 @@
 
 #include <assert.h>
 #include <errno.h>
+#include <string.h>
 
 #include "manager.h"
 #include "hashmap.h"
@@ -91,28 +92,28 @@ int manager_add_job(Manager *m, JobType type, Name *name, JobMode mode, Job **_r
                 goto fail;
 
         if (type == JOB_START || type == JOB_VERIFY_STARTED || type == JOB_RESTART_FINISH) {
-                SET_FOREACH(dep, ret->name->meta.requires, state)
+                SET_FOREACH(dep, ret->name->meta.dependencies[NAME_REQUIRES], state)
                         if ((r = manager_add_job(m, type, dep, mode, NULL)) < 0)
                                 goto fail;
-                SET_FOREACH(dep, ret->name->meta.soft_requires, state)
+                SET_FOREACH(dep, ret->name->meta.dependencies[NAME_SOFT_REQUIRES], state)
                         if ((r = manager_add_job(m, type, dep, JOB_FAIL, NULL)) < 0)
                                 goto fail;
-                SET_FOREACH(dep, ret->name->meta.wants, state)
+                SET_FOREACH(dep, ret->name->meta.dependencies[NAME_WANTS], state)
                         if ((r = manager_add_job(m, type, dep, JOB_FAIL, NULL)) < 0)
                                 goto fail;
-                SET_FOREACH(dep, ret->name->meta.requisite, state)
+                SET_FOREACH(dep, ret->name->meta.dependencies[NAME_REQUISITE], state)
                         if ((r = manager_add_job(m, JOB_VERIFY_STARTED, dep, mode, NULL)) < 0)
                                 goto fail;
-                SET_FOREACH(dep, ret->name->meta.soft_requisite, state)
+                SET_FOREACH(dep, ret->name->meta.dependencies[NAME_SOFT_REQUISITE], state)
                         if ((r = manager_add_job(m, JOB_VERIFY_STARTED, dep, JOB_FAIL, NULL)) < 0)
                                 goto fail;
-                SET_FOREACH(dep, ret->name->meta.conflicts, state)
+                SET_FOREACH(dep, ret->name->meta.dependencies[NAME_CONFLICTS], state)
                         if ((r = manager_add_job(m, type, dep, mode, NULL)) < 0)
                                 goto fail;
 
         } else if (type == JOB_STOP || type == JOB_RESTART || type == JOB_TRY_RESTART) {
 
-                SET_FOREACH(dep, ret->name->meta.required_by, state)
+                SET_FOREACH(dep, ret->name->meta.dependencies[NAME_REQUIRED_BY], state)
                         if ((r = manager_add_job(m, type, dep, mode, NULL)) < 0)
                                 goto fail;
         }
@@ -142,17 +143,18 @@ Name *manager_get_name(Manager *m, const char *name) {
         return hashmap_get(m->names, name);
 }
 
-static int detect_type(Name *name) {
-        char **n;
+static int verify_type(Name *name) {
+        char *n;
+        void *state;
 
         assert(name);
 
-        name->meta.type = _NAME_TYPE_INVALID;
+        /* Checks that all aliases of this name have the same and valid type */
 
-        STRV_FOREACH(n, name->meta.names) {
+        SET_FOREACH(n, name->meta.names, state) {
                 NameType t;
 
-                if ((t = name_type_from_string(*n)) == _NAME_TYPE_INVALID)
+                if ((t = name_type_from_string(n)) == _NAME_TYPE_INVALID)
                         return -EINVAL;
 
                 if (name->meta.type == _NAME_TYPE_INVALID) {
@@ -164,6 +166,9 @@ static int detect_type(Name *name) {
                         return -EINVAL;
         }
 
+        if (name->meta.type == _NAME_TYPE_INVALID)
+                return -EINVAL;
+
         return 0;
 }
 
@@ -209,7 +214,7 @@ static int load(Name *name) {
         if (name->meta.state != NAME_STUB)
                 return 0;
 
-        if ((r = detect_type(name)) < 0)
+        if ((r = verify_type(name)) < 0)
                 return r;
 
         if (name->meta.type == NAME_SERVICE) {
@@ -278,6 +283,7 @@ int manager_load_name(Manager *m, const char *name, Name **_ret) {
         Name *ret;
         NameType t;
         int r;
+        char *n;
 
         assert(m);
         assert(name);
@@ -300,8 +306,14 @@ int manager_load_name(Manager *m, const char *name, Name **_ret) {
 
         ret->meta.type = t;
 
-        if (!(ret->meta.names = strv_new(name, NULL))) {
+        if (!(n = strdup(name))) {
+                name_free(ret);
+                return -ENOMEM;
+        }
+
+        if (set_put(ret->meta.names, n) < 0) {
                 name_free(ret);
+                free(n);
                 return -ENOMEM;
         }
 
diff --git a/manager.h b/manager.h
index 9cd131d5a..64c4c9d14 100644
--- a/manager.h
+++ b/manager.h
@@ -17,6 +17,10 @@ typedef struct Manager Manager;
 struct Manager {
         uint32_t current_job_id;
 
+        /* Note that the set of names we know of is allowed to be
+         * incosistent. However the subset of it that is loaded may
+         * not, and the list of jobs may neither. */
+
         /* Active jobs and names */
         Hashmap *names;  /* name string => Name object n:1 */
         Hashmap *jobs;   /* job id => Job object 1:1 */
diff --git a/name.c b/name.c
index 15e324c5f..f29ce2298 100644
--- a/name.c
+++ b/name.c
@@ -65,6 +65,11 @@ Name *name_new(Manager *m) {
         if (!(n = new0(Name, 1)))
                 return NULL;
 
+        if (!(n->meta.names = set_new(string_hash_func, string_compare_func))) {
+                free(n);
+                return NULL;
+        }
+
         /* Not much initialization happening here at this time */
         n->meta.manager = m;
         n->meta.type = _NAME_TYPE_INVALID;
@@ -78,12 +83,14 @@ Name *name_new(Manager *m) {
 int name_link(Name *n) {
         char **t;
         int r;
+        void *state;
 
         assert(n);
+        assert(!set_isempty(n->meta.names));
         assert(!n->meta.linked);
 
-        STRV_FOREACH(t, n->meta.names)
-                if ((r = hashmap_put(n->meta.manager->names, *t, n)) < 0)
+        SET_FOREACH(t, n->meta.names, state)
+                if ((r = hashmap_put(n->meta.manager->names, t, n)) < 0)
                         goto fail;
 
         if (n->meta.state == NAME_STUB)
@@ -94,43 +101,56 @@ int name_link(Name *n) {
         return 0;
 
 fail:
-        t--;
-        STRV_FOREACH_BACKWARDS(t, n->meta.names)
-                hashmap_remove(n->meta.manager->names, *t);
+        SET_FOREACH(t, n->meta.names, state)
+                assert_se(hashmap_remove(n->meta.manager->names, t) == n);
 
         return r;
 }
 
+static void bidi_set_free(Name *name, Set *s) {
+        void *state;
+        Name *other;
+
+        assert(name);
+        assert(s);
+
+        /* Frees the set and makes sure we are dropped from the
+         * inverse pointers */
+
+        SET_FOREACH(other, s, state) {
+                NameDependency d;
+
+                for (d = 0; d < _NAME_DEPENDENCY_MAX; d++)
+                        set_remove(other->meta.dependencies[d], name);
+        }
+
+        set_free(s);
+}
+
 void name_free(Name *name) {
+        NameDependency d;
+        char *t;
 
         assert(name);
 
         /* Detach from next 'bigger' objects */
-
         if (name->meta.linked) {
                 char **t;
+                void *state;
 
-                STRV_FOREACH(t, name->meta.names)
-                        hashmap_remove(name->meta.manager->names, *t);
+                SET_FOREACH(t, name->meta.names, state)
+                        assert_se(hashmap_remove(name->meta.manager->names, t) == name);
 
-                if (name->meta.job)
-                        job_free(name->meta.job);
+                if (name->meta.state == NAME_STUB)
+                        LIST_REMOVE(Meta, name->meta.manager->load_queue, &name->meta);
         }
 
         /* Free data and next 'smaller' objects */
-
         if (name->meta.job)
                 job_free(name->meta.job);
 
-        /* FIXME: Other names pointing to us should probably drop their refs to us when we get destructed */
-        set_free(name->meta.requires);
-        set_free(name->meta.soft_requires);
-        set_free(name->meta.wants);
-        set_free(name->meta.requisite);
-        set_free(name->meta.soft_requires);
-        set_free(name->meta.conflicts);
-        set_free(name->meta.before);
-        set_free(name->meta.after);
+        for (d = 0; d < _NAME_DEPENDENCY_MAX; d++)
+                bidi_set_free(name, name->meta.dependencies[d]);
 
         switch (name->meta.type) {
 
@@ -169,7 +189,10 @@ void name_free(Name *name) {
         }
 
         free(name->meta.description);
-        strv_free(name->meta.names);
+
+        while ((t = set_steal_first(name->meta.names)))
+                free(t);
+        set_free(name->meta.names);
 
         free(name);
 }
@@ -264,31 +287,76 @@ int name_augment(Name *n) {
 
         assert(n);
 
-        /* Adds in the missing links to make all dependencies both-ways */
+        /* Adds in the missing links to make all dependencies bidirectional */
 
-        SET_FOREACH(other, n->meta.before, state)
-                if ((r = ensure_in_set(&other->meta.after, n) < 0))
+        SET_FOREACH(other, n->meta.dependencies[NAME_BEFORE], state)
+                if ((r = ensure_in_set(&other->meta.dependencies[NAME_AFTER], n) < 0))
                         return r;
-        SET_FOREACH(other, n->meta.after, state)
-                if ((r = ensure_in_set(&other->meta.before, n) < 0))
+        SET_FOREACH(other, n->meta.dependencies[NAME_AFTER], state)
+                if ((r = ensure_in_set(&other->meta.dependencies[NAME_BEFORE], n) < 0))
                         return r;
 
-        SET_FOREACH(other, n->meta.conflicts, state)
-                if ((r = ensure_in_set(&other->meta.conflicts, n) < 0))
+        SET_FOREACH(other, n->meta.dependencies[NAME_CONFLICTS], state)
+                if ((r = ensure_in_set(&other->meta.dependencies[NAME_CONFLICTS], n) < 0))
                         return r;
 
-        SET_FOREACH(other, n->meta.requires, state)
-                if ((r = ensure_in_set(&other->meta.required_by, n) < 0))
+        SET_FOREACH(other, n->meta.dependencies[NAME_REQUIRES], state)
+                if ((r = ensure_in_set(&other->meta.dependencies[NAME_REQUIRED_BY], n) < 0))
                         return r;
-        SET_FOREACH(other, n->meta.soft_requires, state)
-                if ((r = ensure_in_set(&other->meta.required_by, n) < 0))
+        SET_FOREACH(other, n->meta.dependencies[NAME_REQUISITE], state)
+                if ((r = ensure_in_set(&other->meta.dependencies[NAME_REQUIRED_BY], n) < 0))
                         return r;
-        SET_FOREACH(other, n->meta.requisite, state)
-                if ((r = ensure_in_set(&other->meta.required_by, n) < 0))
+
+        SET_FOREACH(other, n->meta.dependencies[NAME_WANTS], state)
+                if ((r = ensure_in_set(&other->meta.dependencies[NAME_WANTED_BY], n) < 0))
                         return r;
-        SET_FOREACH(other, n->meta.soft_requisite, state)
-                if ((r = ensure_in_set(&other->meta.required_by, n) < 0))
+        SET_FOREACH(other, n->meta.dependencies[NAME_SOFT_REQUIRES], state)
+                if ((r = ensure_in_set(&other->meta.dependencies[NAME_WANTED_BY], n) < 0))
+                        return r;
+        SET_FOREACH(other, n->meta.dependencies[NAME_SOFT_REQUISITE], state)
+                if ((r = ensure_in_set(&other->meta.dependencies[NAME_WANTED_BY], n) < 0))
                         return r;
 
         return r;
 }
+
+static int ensure_merge(Set **s, Set *other) {
+
+        if (!other)
+                return 0;
+
+        if (*s)
+                return set_merge(*s, other);
+
+        if (!(*s = set_copy(other)))
+                return -ENOMEM;
+
+        return 0;
+}
+
+int name_merge(Name *name, Name *other) {
+        int r;
+        NameDependency d;
+
+        assert(name);
+        assert(other);
+
+        assert(name->meta.manager == other->meta.manager);
+
+        if (name->meta.type != other->meta.type)
+                return -EINVAL;
+
+        if (other->meta.state != NAME_STUB)
+                return -EINVAL;
+
+        /* Merge names */
+        if ((r = ensure_merge(&name->meta.names, other->meta.names)) < 0)
+                return r;
+
+        /* Merge dependencies */
+        for (d = 0; d < _NAME_DEPENDENCY_MAX; d++)
+                if ((r = ensure_merge(&name->meta.dependencies[d], other->meta.dependencies[d])) < 0)
+                        return r;
+
+        return 0;
+}
diff --git a/name.h b/name.h
index 062c95aa1..696953108 100644
--- a/name.h
+++ b/name.h
@@ -42,24 +42,33 @@ typedef enum NameState {
         NAME_FAILED
 } NameState;
 
-struct Meta {
-        Manager *manager;
-        NameType type;
-        NameState state;
-
-        char **names;
-
+typedef enum NameDependency {
         /* Positive dependencies */
-        Set *requires, *soft_requires, *wants, *requisite, *soft_requisite;
-        Set *required_by;    /* inverse of 'requires', 'soft_requires', 'requisite' and 'soft_requisite' is 'required_by' */
+        NAME_REQUIRES,
+        NAME_SOFT_REQUIRES,
+        NAME_WANTS,
+        NAME_REQUISITE,
+        NAME_SOFT_REQUISITE,
+        NAME_REQUIRED_BY,   /* inverse of 'requires' and 'requisite' is 'required_by' */
+        NAME_WANTED_BY,     /* inverse of 'wants', 'soft_requires' and 'soft_requisite' is 'wanted_by' */
 
         /* Negative dependencies */
-        Set *conflicts;      /* inverse of 'conflicts' is 'conflicts' */
+        NAME_CONFLICTS,     /* inverse of 'conflicts' is 'conflicts' */
 
         /* Order */
-        Set *before, *after; /* inverse of before is after and vice versa */
+        NAME_BEFORE,        /* inverse of before is after and vice versa */
+        NAME_AFTER,
+        _NAME_DEPENDENCY_MAX
+} NameDependency;
+
+struct Meta {
+        Manager *manager;
+        NameType type;
+        NameState state;
+
+        Set *names;
+        Set *dependencies[_NAME_DEPENDENCY_MAX];
 
-        /* Information */
         char *description;
 
         /* If there is something to do with this name, then this is
@@ -265,7 +274,7 @@ bool name_is_valid(const char *n);
 Name *name_new(Manager *m);
 void name_free(Name *name);
 int name_link(Name *name);
-
+int name_merge(Name *name, Name *other);
 int name_augment(Name *n);
 
 #endif