[systemd-devel] [PATCH v2] fstab-generator: add x-systemd.requires and x-systemd.requires-mounts-for

[Lennart Poettering]

On Thu, 14.05.15 13:27, Karel Zak (kzak at redhat.com) wrote:

> +        r = fstab_filter_options(opts, "x-systemd.requires\0", NULL, &arg, NULL);
> +        if (r < 0)
> +                return log_warning_errno(r, "Failed to parse options: %m");
> +        if (r == 0)
> +                return 0;
> +
> +        if (*arg == '/') {
> +                r = unit_name_mangle_with_suffix(arg, UNIT_NAME_NOGLOB, ".mount", &unit);
> +                if (r < 0)
> +                        return log_error_errno(r, "Failed to generate unit name: %m");
> +        } else
> +                unit = arg;

No need to explicitly check *arg for '/', just invoke
unit_name_mangle_with_suffix() immediately, without condition, as it
internally does pretty much the same check anyway. i.e. remove the if
check, remove the second branch, just invoke the function immediatey
and unconditionally, it will already do the right thing...

> +        fprintf(f, "After=%1$s\nRequires=%1$s\n", unit);
> +
> +        if (unit == arg)
> +                unit = NULL;
> +
> +        return 0;
> +}
> +
> +#define REQUIRES_MOUNTS_OPT        "x-systemd.requires-mounts-for="

I'd just use the literal string everywhere. We only use this in this
one file and the macro isn't really that much shorter than the real
string, there's really no need to define a macro for this... 

> +
> +static int write_requires_mounts_for(FILE *f, const char *opts) {
> +        _cleanup_free_ char **optsv = NULL, **paths = NULL, *res = NULL;
> +        char **s;
> +
> +        assert(f);
> +        assert(opts);
> +
> +        optsv = strv_split(opts, ",");
> +        if (!optsv)
> +                return log_oom();
> +
> +        STRV_FOREACH(s, optsv) {
> +                char *arg;
> +
> +                if (!startswith(*s, REQUIRES_MOUNTS_OPT))
> +                        continue;
> +                arg = *s + strlen(REQUIRES_MOUNTS_OPT);
> +                if (!*arg)
> +                        return log_warning("Failed to parse option " REQUIRES_MOUNTS_OPT);
> +                if (!paths)
> +                        paths = strv_new(arg, NULL);
> +                else
> +                        strv_push(&paths, arg);
> +        }

I figure this logic should really be made generic and added to
fstab-util.c or so.

Lennart

-- 
Lennart Poettering, Red Hat