[systemd-commits] fixme Makefile.am src/systemctl.c src/unit.c src/unit.h src/unit-name.c src/unit-name.h
Lennart Poettering
lennart at kemper.freedesktop.org
Tue Oct 5 17:33:48 PDT 2010
Makefile.am | 3 ++-
fixme | 9 ++++-----
src/systemctl.c | 38 +++++++++++++++-----------------------
src/unit-name.c | 31 ++++++-------------------------
src/unit-name.h | 6 +++---
src/unit.c | 22 ++++++++++++++++++++++
src/unit.h | 4 +++-
7 files changed, 55 insertions(+), 58 deletions(-)
New commits:
commit 71fad6751434f06485a744d41be2d807303c1184
Author: Lennart Poettering <lennart at poettering.net>
Date: Wed Oct 6 02:33:40 2010 +0200
systemctl: require correctly formed unit names when enabling units
diff --git a/Makefile.am b/Makefile.am
index c06f1ec..4307db1 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -700,7 +700,8 @@ systemctl_SOURCES = \
src/sd-daemon.c \
src/cgroup-show.c \
src/cgroup-util.c \
- src/exit-status.c
+ src/exit-status.c \
+ src/unit-name.c
systemctl_CFLAGS = \
$(AM_CFLAGS) \
diff --git a/fixme b/fixme
index fd75631..c4582d3 100644
--- a/fixme
+++ b/fixme
@@ -4,9 +4,6 @@ v11:
* verify ordering of random-seed-load and base.target!
-* systemctl enable /lib/systemd/system/?*.service
- creates wrong subdirs in /etc/ssytemd/system/ which prevent further bootup
-
later:
* do not throw error when .service file is linked to /dev/null
@@ -97,8 +94,6 @@ later:
* document locale.conf, vconsole.conf and possibly the tempfiles.d and modules-load.d mechanism.
-* when /proc/self/mountinfo is not parsable, proceed with next line
-
* beefed up tmpwatch that reads tmpfiles.d
* /lib/systemd/system/systemd-readahead-replay.service
@@ -109,6 +104,10 @@ later:
* Restart=on-failure and Restart=on-abort
+* kill sessions on shutdown
+
+* when processes remain in a service even though the start command failed enter active
+
External:
* place /etc/inittab with explaining blurb.
diff --git a/src/systemctl.c b/src/systemctl.c
index 1f2b43b..45249aa 100644
--- a/src/systemctl.c
+++ b/src/systemctl.c
@@ -53,6 +53,7 @@
#include "exit-status.h"
#include "bus-errors.h"
#include "build.h"
+#include "unit-name.h"
static const char *arg_type = NULL;
static char **arg_property = NULL;
@@ -3237,28 +3238,16 @@ static void install_info_hashmap_free(Hashmap *m) {
hashmap_free(m);
}
-static bool unit_name_valid(const char *name) {
-
- /* This is a minimal version of unit_name_valid() from
- * unit-name.c */
-
- if (!*name)
- return false;
-
- if (ignore_file(name))
- return false;
-
- return true;
-}
-
static int install_info_add(const char *name) {
InstallInfo *i;
int r;
assert(will_install);
- if (!unit_name_valid(name))
+ if (!unit_name_is_valid_no_type(name)) {
+ log_warning("Unit name %s is not a valid unit name.", name);
return -EINVAL;
+ }
if (hashmap_get(have_installed, name) ||
hashmap_get(will_install, name))
@@ -3310,11 +3299,13 @@ static int config_parse_also(
if (!(n = strndup(w, l)))
return -ENOMEM;
- r = install_info_add(n);
- free(n);
-
- if (r < 0)
+ if ((r = install_info_add(n)) < 0) {
+ log_warning("Cannot install unit %s: %s", n, strerror(-r));
+ free(n);
return r;
+ }
+
+ free(n);
}
return 0;
@@ -3639,7 +3630,7 @@ static int install_info_symlink_alias(const char *verb, InstallInfo *i, const ch
STRV_FOREACH(s, i->aliases) {
- if (!unit_name_valid(*s)) {
+ if (!unit_name_is_valid_no_type(*s)) {
log_error("Invalid name %s.", *s);
r = -EINVAL;
goto finish;
@@ -3658,7 +3649,6 @@ static int install_info_symlink_alias(const char *verb, InstallInfo *i, const ch
if (streq(verb, "disable"))
rmdir_parents(alias_path, config_path);
}
-
r = 0;
finish:
@@ -3677,7 +3667,7 @@ static int install_info_symlink_wants(const char *verb, InstallInfo *i, const ch
assert(config_path);
STRV_FOREACH(s, i->wanted_by) {
- if (!unit_name_valid(*s)) {
+ if (!unit_name_is_valid_no_type(*s)) {
log_error("Invalid name %s.", *s);
r = -EINVAL;
goto finish;
@@ -3840,8 +3830,10 @@ static int enable_unit(DBusConnection *bus, char **args, unsigned n) {
}
for (j = 1; j < n; j++)
- if ((r = install_info_add(args[j])) < 0)
+ if ((r = install_info_add(args[j])) < 0) {
+ log_warning("Cannot install unit %s: %s", args[j], strerror(-r));
goto finish;
+ }
while ((i = hashmap_first(will_install))) {
int q;
diff --git a/src/unit-name.c b/src/unit-name.c
index 868d13e..0e86b55 100644
--- a/src/unit-name.c
+++ b/src/unit-name.c
@@ -21,8 +21,9 @@
#include <errno.h>
#include <string.h>
+#include <assert.h>
-#include "unit.h"
+#include "util.h"
#include "unit-name.h"
#define VALID_CHARS \
@@ -31,20 +32,7 @@
"ABCDEFGHIJKLMNOPQRSTUVWXYZ" \
":-_.\\"
-UnitType unit_name_to_type(const char *n) {
- UnitType t;
-
- assert(n);
-
- for (t = 0; t < _UNIT_TYPE_MAX; t++)
- if (endswith(n, unit_vtable[t]->suffix))
- return t;
-
- return _UNIT_TYPE_INVALID;
-}
-
-bool unit_name_is_valid(const char *n) {
- UnitType t;
+bool unit_name_is_valid_no_type(const char *n) {
const char *e, *i, *at;
/* Valid formats:
@@ -58,13 +46,8 @@ bool unit_name_is_valid(const char *n) {
if (strlen(n) >= UNIT_NAME_MAX)
return false;
- t = unit_name_to_type(n);
- if (t < 0 || t >= _UNIT_TYPE_MAX)
- return false;
-
- assert_se(e = strrchr(n, '.'));
-
- if (e == n)
+ e = strrchr(n, '.');
+ if (!e || e == n)
return false;
for (i = n, at = NULL; i < e; i++) {
@@ -167,7 +150,7 @@ char *unit_name_change_suffix(const char *n, const char *suffix) {
size_t a, b;
assert(n);
- assert(unit_name_is_valid(n));
+ assert(unit_name_is_valid_no_type(n));
assert(suffix);
assert_se(e = strrchr(n, '.'));
@@ -190,7 +173,6 @@ char *unit_name_build(const char *prefix, const char *instance, const char *suff
assert(unit_prefix_is_valid(prefix));
assert(!instance || unit_instance_is_valid(instance));
assert(suffix);
- assert(unit_name_to_type(suffix) >= 0);
if (!instance)
return strappend(prefix, suffix);
@@ -226,7 +208,6 @@ char *unit_name_build_escape(const char *prefix, const char *instance, const cha
assert(prefix);
assert(suffix);
- assert(unit_name_to_type(suffix) >= 0);
/* Takes a arbitrary string for prefix and instance plus a
* suffix and makes a nice string suitable as unit name of it,
diff --git a/src/unit-name.h b/src/unit-name.h
index 3ad25ba..a752f3a 100644
--- a/src/unit-name.h
+++ b/src/unit-name.h
@@ -22,15 +22,15 @@
along with systemd; If not, see <http://www.gnu.org/licenses/>.
***/
-#include "unit.h"
+#include <stdbool.h>
-UnitType unit_name_to_type(const char *n);
+#define UNIT_NAME_MAX 256
int unit_name_to_instance(const char *n, char **instance);
char* unit_name_to_prefix(const char *n);
char* unit_name_to_prefix_and_instance(const char *n);
-bool unit_name_is_valid(const char *n);
+bool unit_name_is_valid_no_type(const char *n);
bool unit_prefix_is_valid(const char *p);
bool unit_instance_is_valid(const char *i);
diff --git a/src/unit.c b/src/unit.c
index fefa0eb..71ef2a7 100644
--- a/src/unit.c
+++ b/src/unit.c
@@ -2207,6 +2207,28 @@ bool unit_pending_active(Unit *u) {
return false;
}
+UnitType unit_name_to_type(const char *n) {
+ UnitType t;
+
+ assert(n);
+
+ for (t = 0; t < _UNIT_TYPE_MAX; t++)
+ if (endswith(n, unit_vtable[t]->suffix))
+ return t;
+
+ return _UNIT_TYPE_INVALID;
+}
+
+bool unit_name_is_valid(const char *n) {
+ UnitType t;
+
+ t = unit_name_to_type(n);
+ if (t < 0 || t >= _UNIT_TYPE_MAX)
+ return false;
+
+ return unit_name_is_valid_no_type(n);
+}
+
static const char* const unit_load_state_table[_UNIT_LOAD_STATE_MAX] = {
[UNIT_STUB] = "stub",
[UNIT_LOADED] = "loaded",
diff --git a/src/unit.h b/src/unit.h
index 47a1eb6..afae580 100644
--- a/src/unit.h
+++ b/src/unit.h
@@ -39,7 +39,6 @@ typedef enum UnitDependency UnitDependency;
#include "socket-util.h"
#include "execute.h"
-#define UNIT_NAME_MAX 256
#define DEFAULT_TIMEOUT_USEC (60*USEC_PER_SEC)
#define DEFAULT_RESTART_USEC (100*USEC_PER_MSEC)
@@ -504,6 +503,9 @@ bool unit_pending_active(Unit *u);
int unit_add_default_target_dependency(Unit *u, Unit *target);
+UnitType unit_name_to_type(const char *n);
+bool unit_name_is_valid(const char *n);
+
const char *unit_load_state_to_string(UnitLoadState i);
UnitLoadState unit_load_state_from_string(const char *s);
More information about the systemd-commits
mailing list