[systemd-devel] [PATCH] fstab-generator: Honor usr=, usrfstype= and usrflags=

[Lennart Poettering]

On Wed, 24.09.14 22:08, Tobias Hunger (tobias.hunger at gmail.com) wrote:

The patch is line-broken, please resend non-linebroken version!

> 
> +static int add_usr_mount(void) {
> +        _cleanup_free_ char *what = NULL;
> +        const char *opts;
> +
> +        if (!arg_usr_what && !arg_usr_fstype && !arg_usr_options)
> +                return 0;
> +
> +        if (arg_root_what && !arg_usr_what)
> +                arg_usr_what = strdup(arg_root_what);

Hmm, what's the rationale here? I mean, then we'd mount the same stuff
to /usr that we already mount to / if one field is missing, is that
really disarable? Maybe better do a warning instead?

> +
> +        if (arg_root_fstype && !arg_usr_fstype)
> +                arg_usr_fstype = strdup(arg_root_fstype);
> +
> +        if (arg_root_options && !arg_usr_options)
> +                arg_usr_options = strdup(arg_root_options);

OOM checks missing for this.

> +        } else if (streq(key, "usr") && value) {
> +
> +                free(arg_usr_what);
> +                arg_usr_what = strdup(value);
> +                if (!arg_usr_what)
> +                        return log_oom();

We recently added free_and_strdup(), which should make the four lines
above shorter:

if (free_and_strdup(&arg_usr_what, value) < 0)
        return log_oom();

> +
> +        } else if (streq(key, "usrfstype") && value) {
> +
> +                free(arg_usr_fstype);
> +                arg_usr_fstype = strdup(value);
> +                if (!arg_usr_fstype)
> +                        return log_oom();

Similar.

Otherwise looks good to me, but I'd really like Harald Hoyer's
feedback on this, before we merge this. I am not entirely sure what
our strategy here is precisely, and I think dracut might already have
something here.

Harald, can you comment?

Lennart

-- 
Lennart Poettering, Red Hat