On Mon, 15.07.13 15:22, Maciej Wereski (m.were...@partner.samsung.com) wrote:
> + <varlistentry> > + <term><varname>t</varname></term> > + <listitem><para>Set extended > + attributes on item. It should be > + used with conjunction with > other Well, I'd use the wording "may be used", not "should be used" here... > + if (!i->xattrs) > + return 0; We usually use strv_isempty() for that, which checks a bit more, but this doesn't really matter in this case... > + STRV_FOREACH(x, i->xattrs) { > + value = *x; > + name = strsep(&value, "="); I'd really prefer if we didn't corrupt the string here. Maybe use strv_split_quoted() here? That handles all the values for you anyway... > + n = strlen(value); > + if (i->type == CREATE_SYMLINK) { > + if (lsetxattr(path, name, value, n+1, 0) < 0) { > + log_error("Setting extended attribute %s=%s > on symlink %s failed: %m", name, value, path); > + free(value); > + return -errno; > + } > + } > + else if (setxattr(path, name, value, n+1, 0) < 0) { > + log_error("Setting extended attribute %s=%s on %s > failed: %m", name, value, path); > + free(value); > + return -errno; > + } > + free(value); Hmm, the two if branches look like something you could combine further: if (i->type == CREATE_SYMLINK) r = lsetxattr(...); else r = setxattr(...); And note that log_error() guarantees that errno stays untouched. > + if (i->xattrs) { > + r = item_set_xattrs(i, i->path); > + if (r < 0) > + return r; > + } Checking i->xattrs outside and inside the function is redundand, no? > + for (n = 0; n < strv_length(tmp); ++n) { > + len = strlen(tmp[n]); > + strncpy(xattr, tmp[n], len+1); > + p = strchr(xattr, '='); > + if (!p) { > + log_error("%s: Attribute has incorrect format.", > i->path); > + return -EBADMSG; > + } > + if (p[1] == '\"') { > + while (true) { > + if (!p) > + p = tmp[n]; > + else > + p += 2; > + p = strchr(p, '\"'); > + if (p && xattr[p-xattr-1] != '\\') > + break; > + p = NULL; > + ++n; > + if (n == strv_length(tmp)) > + break; > + len += strlen(tmp[n]) + 1; > + strncat(xattr, " ", 1); > + strncat(xattr, tmp[n], len); > + } > + } > + strstrip(xattr); > + f = i->xattrs; > + i->xattrs = strv_append(i->xattrs, xattr); > + if (!i->xattrs){ > + strv_free(f); > + return log_oom(); > + } For this stuf I'd really prefer using one of our already existing quoting APIs, like strv_spit_quoted() or FOREACH_WORD_QUOTED or so. > int r = 0, k; > _cleanup_globfree_ glob_t g = {}; > @@ -674,6 +799,12 @@ static int create_item(Item *i) { > if (r < 0) > return r; > > + if (i->xattrs) { > + r = item_set_xattrs(i, i->path); > + if (r < 0) > + return r; > + } > + item_set_xattrs already checks i->xattrs for empty anyway, so this could be shortened quite a bit (as above and a couple of more times) > +static int copy_item_contents(Item *dest, const Item *src) { Hmm, not following why you need to copy any items ever. Either you add a new item, or you patch the existing one, but why ever copy one? Lennart -- Lennart Poettering - Red Hat, Inc. _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel