Re: [systemd-devel] [PATCH v2, ping?] tmpfiles, man: Add xattr support to tmpfiles
It was 2013-07-15 pon 15:22, when Maciej Wereski wrote: diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml index 519f9bc..92157b5 100644 --- a/man/tmpfiles.d.xml +++ b/man/tmpfiles.d.xml @@ -229,6 +229,21 @@ L/tmp/foobar ---- /dev/null/programlisting place of normal path names./para/listitem /varlistentry + +varlistentry +termvarnamet/varname/term +listitemparaSet extended +attributes on item. It should be +used with conjunction with other in conjunction with -- Łukasz Stelmach Samsung RD Institute Poland Samsung Electronics ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH v2, ping?] tmpfiles, man: Add xattr support to tmpfiles
Hello,
16.07.2013 at 00:31 Lennart Poettering lenn...@poettering.net wrote:
+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...
You mean strv_split() (I'm splitting by =)? This has one issue: it
splits by all separator occurrences and I need to split after first one.
If corrupting string is the issue, I can make a copy of it. If you prefer
strv_split(), then I can just join if strv_length 2.
+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.
Well, I've tried it in the beginning, but in doesn't work properly in this
case. split_quoted() expects quote on the beginning of a string (ignoring
whitespace occurrences). If there's no such case string will be split using
whitespace. Example of extended attribute with quotes:
user.test=This will \ fail
So how would you like this case to be solved?
regards,
Maciej
--
Maciej Wereski
Samsung RD Institute Poland
Samsung Electronics
m.were...@partner.samsung.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH v2, ping?] tmpfiles, man: Add xattr support to tmpfiles
On Thu, 18.07.13 17:09, Maciej Wereski (m.were...@partner.samsung.com) wrote:
Hello,
16.07.2013 at 00:31 Lennart Poettering lenn...@poettering.net wrote:
+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...
You mean strv_split() (I'm splitting by =)? This has one issue: it
splits by all separator occurrences and I need to split after first one.
If corrupting string is the issue, I can make a copy of it. If you prefer
strv_split(), then I can just join if strv_length 2.
Oh, true.
Given that this appears to be recurring theme I have now added a
split_pair() call for you. It will split a string up at a specified
separator into a pair. You could then pass the second argument into
unquote() and that's all you need?
Could you please rework your patch with that?
Lennart
--
Lennart Poettering - Red Hat, Inc.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH v2, ping?] tmpfiles, man: Add xattr support to tmpfiles
On Mon, 15.07.13 15:22, Maciej Wereski (m.were...@partner.samsung.com) wrote:
+varlistentry
+termvarnamet/varname/term
+listitemparaSet 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
