On Thu, 13.11.14 09:11, Maciej Wereski (m.were...@partner.samsung.com) wrote:
Sorry for the late review.
> +static int get_xattrs_from_arg(Item *i) {
> + char *xattr;
> + const char *p;
> + int n;
> +
> + assert(i);
> + if (i->type != SET_XATTR)
> + return 0;
> +
> + if (!i->argument) {
> + log_error("%s: Argument can't be empty!", i->path);
> + return -EBADMSG;
> + }
> + p = i->argument;
> +
> + while ((n = unquote_first_word(&p, &xattr, false)) > 0) {
> + if (strv_consume(&i->xattrs, xattr) < 0)
> + return log_oom();
> + }
> +
> + return n;
I think we really should parse the xattrs into key and value at the
time of parsing, not at the time of applying things. If something
parses incorrectly this should be noted before we do actually do
anything.
i think you can even stick to an strv for this, and simply store key
and value alternating in the list. We have STRV_FOREACH_PAIR in place
to iterate through such a list of pairs.
> - r = hashmap_put(h, i->path, i);
> - if (r < 0) {
> - log_error("Failed to insert item %s: %s", i->path,
> strerror(-r));
> - return r;
> + if (i->type == SET_XATTR) {
> + char **tmp;
> + r = get_xattrs_from_arg(i);
> + if (r < 0)
> + return r;
> + tmp = existing->xattrs;
> + r = strv_extend_strv(&existing->xattrs, i->xattrs);
> + if (r < 0) {
> + strv_free(tmp);
> + return log_oom();
> + }
strv_extend_strv() actually realloc()s the existing array, you are
*not* supposed to free the old version!
Lennart
--
Lennart Poettering, Red Hat
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
