[systemd-devel] [PATCH 5/6] Add trivial templated args to [Install] WantedBy and RequiredBy fields

[AvataR]

>> +char *name_printf(char * prefix, char * instance, char* format) {

> We don't need to export this, do we?

Don't know. We don't need this function to be exported, but I have no
idea about the policy of using/definition of such functions in systemd.

> Hmm, specifier_string() already handles NULL strings, and returns the
> empty string for them. Isn't that good enough for this case? Is
> returning the unreplaced %p, resp. %i really nicer in that case?

The idea was: if template parameters pushed to non-templated unit, then
Ivalid argument error will be raised by unit_name_is_valid function. In
other case -- unit not found.  IMO the second one is irrelevant.

> unit_name_to_instance(i->name, &instance);
> We always need to check for OOM for calls like this.

Yes, this should be fixed. How about this one?

diff --git a/src/shared/install.c b/src/shared/install.c
index a9d75f3..83ea06a 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -36,6 +36,7 @@
 #include "install.h"
 #include "conf-parser.h"
 #include "conf-files.h"
+#include "specifier.h"
 
 typedef struct {
         char *name;
@@ -51,6 +52,30 @@ typedef struct {
         Hashmap *have_installed;
 } InstallContext;
 
+static char *name_printf(char * prefix, char * instance, char* format) {
+
+        /*
+         * This will use the passed string as format string and
+         * replace the following specifiers, if any:
+         *
+         * %p: the unit prefix                         (foo)
+         * %i: the instance                            (bar)
+         */
+
+        char empty_prefix[]   = "%p";
+        char empty_instance[] = "%i";
+
+        const Specifier table[] = {
+                { 'p', specifier_string, prefix   ? : empty_prefix},
+                { 'i', specifier_string, instance ? : empty_instance},
+                { 0, NULL, NULL }
+        };
+
+        assert(format);
+
+        return specifier_printf(format, table, NULL);
+}
+
 static int lookup_paths_init_from_scope(LookupPaths *paths, UnitFileScope scope) {
         assert(paths);
         assert(scope >= 0);
@@ -1271,13 +1296,24 @@ static int install_info_symlink_wants(
 
         STRV_FOREACH(s, i->wanted_by) {
                 char *path;
+                char *instance = NULL;
+                char *prefix   = NULL;
+                char *dst      = NULL;
 
-                if (!unit_name_is_valid(*s, true)) {
+                r = unit_name_to_instance(i->name, &instance);
+                if (r != 0)
+                        break;
+
+                prefix = unit_name_to_prefix(i->name);
+
+                dst = name_printf(prefix, instance, *s);
+
+                if (!unit_name_is_valid(dst, true)) {
                         r = -EINVAL;
                         continue;
                 }
 
-                if (asprintf(&path, "%s/%s.wants/%s", config_path, *s, i->name) < 0)
+                if (asprintf(&path, "%s/%s.wants/%s", config_path, dst, i->name) < 0)
                         return -ENOMEM;
 
                 q = create_symlink(i->path, path, force, changes, n_changes);
@@ -1305,13 +1341,24 @@ static int install_info_symlink_requires(
 
         STRV_FOREACH(s, i->required_by) {
                 char *path;
+                char *instance = NULL;
+                char *prefix   = NULL;
+                char *dst      = NULL;
+
+                r = unit_name_to_instance(i->name, &instance);
+                if (r != 0)
+                        break;
+
+                prefix = unit_name_to_prefix(i->name);
+
+                dst = name_printf(prefix, instance, *s);
 
-                if (!unit_name_is_valid(*s, true)) {
+                if (!unit_name_is_valid(dst, true)) {
                         r = -EINVAL;
                         continue;
                 }
 
-                if (asprintf(&path, "%s/%s.requires/%s", config_path, *s, i->name) < 0)
+                if (asprintf(&path, "%s/%s.requires/%s", config_path, dst, i->name) < 0)
                         return -ENOMEM;
 
                 q = create_symlink(i->path, path, force, changes, n_changes);