On Fri, 15.05.15 12:41, Karel Zak (k...@redhat.com) wrote:
> +static int write_requires_after(FILE *f, const char *opts) {
> + _cleanup_free_ char *arg = NULL, *unit = NULL;
> + int r;
> +
> + assert(f);
> + assert(opts);
> +
> + 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;
Shouldn't this also use the new fstab_extract_values() call? I mean
people might want to specify multiple of these, too?
>
> +int fstab_extract_values(const char *opts, const char *name, char ***values)
> +{
> + _cleanup_strv_free_ char **optsv = NULL, **res = NULL;
> + char **s;
Minor nitpick: we usually place the opening brackets on the same line as the
function name:
int fstab_extract_values(...) {
rather than:
int fstab_extract_values(...)
{
> +
> + assert(opts);
> + assert(name);
> + assert(values);
> +
> + optsv = strv_split(opts, ",");
> + if (!optsv)
> + return -ENOMEM;
> +
> + STRV_FOREACH(s, optsv) {
> + char *arg;
> +
> + arg = startswith(*s, name);
> + if (!arg || *arg != '=')
> + continue;
> +
> + arg = strdup(arg + 1);
> + if (!arg)
> + return -ENOMEM;
> + if (!res)
> + res = strv_new(arg, NULL);
> + else
> + strv_push(&res, arg);
This misses an OOM check.
Also, if res is NULL strv_push() will already create a new array
implicitly for you. For most strv_xyz() calls we consider a NULL strv
array identical to an empty array...
This basically means you can drop the explicit strv_new(), and just
leave strv_push() in place.
That said, you can also drop the strdup(arg + 1) before, and simply
make use of strv_extend() which internally does the strdup() and then
uses strv_push() to add it to the list...
Lennart
--
Lennart Poettering, Red Hat
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
