From df7c4eb62a22a5b131007d98ee49f57f6f95483e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 24 Oct 2019 14:09:11 +0200 Subject: [PATCH] various tools: be more explicit when a glob is passed when not supported See https://bugzilla.redhat.com/show_bug.cgi?id=1763488: when we say that 'foo@*.service' is not a valid unit name, this is not clear enough. Let's include the name of the operation that does not support globbing in the error message: $ build/systemctl enable 'foo@*.service' Glob pattern passed to enable, but globs are not supported for this. Invalid unit name "foo@*.service" escaped as "foo@\x2a.service". ... --- src/analyze/analyze-security.c | 2 +- src/basic/unit-name.c | 25 +++++++++++++++---------- src/basic/unit-name.h | 4 ++-- src/fstab-generator/fstab-generator.c | 2 +- src/nspawn/nspawn-register.c | 4 ++-- src/run/run.c | 20 +++++++++++++++----- src/systemctl/systemctl.c | 25 ++++++++++++++----------- 7 files changed, 50 insertions(+), 32 deletions(-) diff --git a/src/analyze/analyze-security.c b/src/analyze/analyze-security.c index 848aeaed80..ca023ea807 100644 --- a/src/analyze/analyze-security.c +++ b/src/analyze/analyze-security.c @@ -2093,7 +2093,7 @@ int analyze_security(sd_bus *bus, char **units, AnalyzeSecurityFlags flags) { fflush(stdout); } - r = unit_name_mangle_with_suffix(*i, 0, ".service", &mangled); + r = unit_name_mangle(*i, 0, &mangled); if (r < 0) return log_error_errno(r, "Failed to mangle unit name '%s': %m", *i); diff --git a/src/basic/unit-name.c b/src/basic/unit-name.c index bcd01f8515..3e37e34325 100644 --- a/src/basic/unit-name.c +++ b/src/basic/unit-name.c @@ -597,10 +597,10 @@ static bool do_escape_mangle(const char *f, bool allow_globs, char *t) { * * If @allow_globs, globs characters are preserved. Otherwise, they are escaped. */ -int unit_name_mangle_with_suffix(const char *name, UnitNameMangle flags, const char *suffix, char **ret) { +int unit_name_mangle_with_suffix(const char *name, const char *operation, UnitNameMangle flags, const char *suffix, char **ret) { char *s; int r; - bool mangled; + bool mangled, suggest_escape = true; assert(name); assert(suffix); @@ -617,10 +617,14 @@ int unit_name_mangle_with_suffix(const char *name, UnitNameMangle flags, const c goto good; /* Already a fully valid globbing expression? If so, no mangling is necessary either... */ - if ((flags & UNIT_NAME_MANGLE_GLOB) && - string_is_glob(name) && - in_charset(name, VALID_CHARS_GLOB)) - goto good; + if (string_is_glob(name) && in_charset(name, VALID_CHARS_GLOB)) { + if (flags & UNIT_NAME_MANGLE_GLOB) + goto good; + log_full(flags & UNIT_NAME_MANGLE_WARN ? LOG_NOTICE : LOG_DEBUG, + "Glob pattern passed%s%s, but globs are not supported for this.", + operation ? " " : "", operation ?: ""); + suggest_escape = false; + } if (is_device_path(name)) { r = unit_name_from_path(name, ".device", ret); @@ -645,11 +649,12 @@ int unit_name_mangle_with_suffix(const char *name, UnitNameMangle flags, const c mangled = do_escape_mangle(name, flags & UNIT_NAME_MANGLE_GLOB, s); if (mangled) log_full(flags & UNIT_NAME_MANGLE_WARN ? LOG_NOTICE : LOG_DEBUG, - "Invalid unit name \"%s\" was escaped as \"%s\" (maybe you should use systemd-escape?)", - name, s); + "Invalid unit name \"%s\" escaped as \"%s\"%s.", + name, s, + suggest_escape ? " (maybe you should use systemd-escape?)" : ""); - /* Append a suffix if it doesn't have any, but only if this is not a glob, so that we can allow "foo.*" as a - * valid glob. */ + /* Append a suffix if it doesn't have any, but only if this is not a glob, so that we can allow + * "foo.*" as a valid glob. */ if ((!(flags & UNIT_NAME_MANGLE_GLOB) || !string_is_glob(s)) && unit_name_to_type(s) < 0) strcat(s, suffix); diff --git a/src/basic/unit-name.h b/src/basic/unit-name.h index ddcfc1b349..4a6f64cc1c 100644 --- a/src/basic/unit-name.h +++ b/src/basic/unit-name.h @@ -52,10 +52,10 @@ typedef enum UnitNameMangle { UNIT_NAME_MANGLE_WARN = 1 << 1, } UnitNameMangle; -int unit_name_mangle_with_suffix(const char *name, UnitNameMangle flags, const char *suffix, char **ret); +int unit_name_mangle_with_suffix(const char *name, const char *operation, UnitNameMangle flags, const char *suffix, char **ret); static inline int unit_name_mangle(const char *name, UnitNameMangle flags, char **ret) { - return unit_name_mangle_with_suffix(name, flags, ".service", ret); + return unit_name_mangle_with_suffix(name, NULL, flags, ".service", ret); } bool service_unit_name_is_valid(const char *name); diff --git a/src/fstab-generator/fstab-generator.c b/src/fstab-generator/fstab-generator.c index 026a25f575..90e7237576 100644 --- a/src/fstab-generator/fstab-generator.c +++ b/src/fstab-generator/fstab-generator.c @@ -227,7 +227,7 @@ static int write_dependency(FILE *f, const char *opts, STRV_FOREACH(s, names) { char *x; - r = unit_name_mangle_with_suffix(*s, 0, ".mount", &x); + r = unit_name_mangle_with_suffix(*s, "as dependency", 0, ".mount", &x); if (r < 0) return log_error_errno(r, "Failed to generate unit name: %m"); r = strv_consume(&units, x); diff --git a/src/nspawn/nspawn-register.c b/src/nspawn/nspawn-register.c index 8e2c329665..b2d931a8d3 100644 --- a/src/nspawn/nspawn-register.c +++ b/src/nspawn/nspawn-register.c @@ -258,7 +258,7 @@ int allocate_scope( if (r < 0) return log_error_errno(r, "Could not watch job: %m"); - r = unit_name_mangle_with_suffix(machine_name, 0, ".scope", &scope); + r = unit_name_mangle_with_suffix(machine_name, "as machine name", 0, ".scope", &scope); if (r < 0) return log_error_errno(r, "Failed to mangle scope name: %m"); @@ -350,7 +350,7 @@ int terminate_scope( _cleanup_free_ char *scope = NULL; int r; - r = unit_name_mangle_with_suffix(machine_name, 0, ".scope", &scope); + r = unit_name_mangle_with_suffix(machine_name, "to terminate", 0, ".scope", &scope); if (r < 0) return log_error_errno(r, "Failed to mangle scope name: %m"); diff --git a/src/run/run.c b/src/run/run.c index e7d8b30a7a..afa9d14af3 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -641,7 +641,9 @@ static int transient_cgroup_set_properties(sd_bus_message *m) { if (!isempty(arg_slice)) { _cleanup_free_ char *slice = NULL; - r = unit_name_mangle_with_suffix(arg_slice, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".slice", &slice); + r = unit_name_mangle_with_suffix(arg_slice, "as slice", + arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, + ".slice", &slice); if (r < 0) return log_error_errno(r, "Failed to mangle name '%s': %m", arg_slice); @@ -1112,7 +1114,9 @@ static int start_transient_service( } if (arg_unit) { - r = unit_name_mangle_with_suffix(arg_unit, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".service", &service); + r = unit_name_mangle_with_suffix(arg_unit, "as unit", + arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, + ".service", &service); if (r < 0) return log_error_errno(r, "Failed to mangle unit name: %m"); } else { @@ -1355,7 +1359,9 @@ static int start_transient_scope(sd_bus *bus) { return log_oom(); if (arg_unit) { - r = unit_name_mangle_with_suffix(arg_unit, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".scope", &scope); + r = unit_name_mangle_with_suffix(arg_unit, "as unit", + arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, + ".scope", &scope); if (r < 0) return log_error_errno(r, "Failed to mangle scope name: %m"); } else { @@ -1530,11 +1536,15 @@ static int start_transient_trigger( break; default: - r = unit_name_mangle_with_suffix(arg_unit, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".service", &service); + r = unit_name_mangle_with_suffix(arg_unit, "as unit", + arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, + ".service", &service); if (r < 0) return log_error_errno(r, "Failed to mangle unit name: %m"); - r = unit_name_mangle_with_suffix(arg_unit, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, suffix, &trigger); + r = unit_name_mangle_with_suffix(arg_unit, "as trigger", + arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, + suffix, &trigger); if (r < 0) return log_error_errno(r, "Failed to mangle unit name: %m"); diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 05013484ad..3fabbb665c 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -761,10 +761,7 @@ static int expand_names(sd_bus *bus, char **names, const char* suffix, char ***r char *t; UnitNameMangle options = UNIT_NAME_MANGLE_GLOB | (arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN); - if (suffix) - r = unit_name_mangle_with_suffix(*name, options, suffix, &t); - else - r = unit_name_mangle(*name, options, &t); + r = unit_name_mangle_with_suffix(*name, NULL, options, suffix ?: ".service", &t); if (r < 0) return log_error_errno(r, "Failed to mangle name: %m"); @@ -2182,7 +2179,9 @@ static int set_default(int argc, char *argv[], void *userdata) { assert(argc >= 2); assert(argv); - r = unit_name_mangle_with_suffix(argv[1], arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".target", &unit); + r = unit_name_mangle_with_suffix(argv[1], "set-default", + arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, + ".target", &unit); if (r < 0) return log_error_errno(r, "Failed to mangle unit name: %m"); @@ -6565,7 +6564,7 @@ static int enable_sysv_units(const char *verb, char **args) { return r; } -static int mangle_names(char **original_names, char ***mangled_names) { +static int mangle_names(const char *operation, char **original_names, char ***mangled_names) { char **i, **l, **name; int r; @@ -6585,7 +6584,9 @@ static int mangle_names(char **original_names, char ***mangled_names) { return log_oom(); } } else { - r = unit_name_mangle(*name, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, i); + r = unit_name_mangle_with_suffix(*name, operation, + arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, + ".service", i); if (r < 0) { *i = NULL; strv_free(l); @@ -6696,7 +6697,7 @@ static int enable_unit(int argc, char *argv[], void *userdata) { if (!argv[1]) return 0; - r = mangle_names(strv_skip(argv, 1), &names); + r = mangle_names("to enable", strv_skip(argv, 1), &names); if (r < 0) return r; @@ -6923,11 +6924,13 @@ static int add_dependency(int argc, char *argv[], void *userdata) { if (!argv[1]) return 0; - r = unit_name_mangle_with_suffix(argv[1], arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".target", &target); + r = unit_name_mangle_with_suffix(argv[1], "as target", + arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, + ".target", &target); if (r < 0) return log_error_errno(r, "Failed to mangle unit name: %m"); - r = mangle_names(strv_skip(argv, 2), &names); + r = mangle_names("as dependency", strv_skip(argv, 2), &names); if (r < 0) return r; @@ -7113,7 +7116,7 @@ static int unit_is_enabled(int argc, char *argv[], void *userdata) { char **name; int r; - r = mangle_names(strv_skip(argv, 1), &names); + r = mangle_names("to check", strv_skip(argv, 1), &names); if (r < 0) return r;