[systemd-devel] [PATCHv4] tmpfiles, man: Add xattr support to tmpfiles

[Lennart Poettering]

On Thu, 08.08.13 13:54, Maciej Wereski (m.wereski at partner.samsung.com) wrote:

> +                value = strreplace(tmp, "\\\"", "\"");
> +                if (!value)
> +                        return log_oom();

This looks like a job for cunescape() or so?

> +                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);
> +                                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);
> +                        return -errno;
> +                }
> +        }
> +        return 0;
> +#else
> +        (void)i;
> +        (void)path;
> +        log_error("Setting extended attributes requested, but tmpfiles was compiled without XATTR support!");
> +        return -ENOTSUP;

If I follow the control flow correctly then this message would be
generated for *all* entries if XATTR support is not compiled in,
regardless whether xattrs shall actually be set... This part should come
after the early-return check whether the xattr list is actually empty...

> +        xattr = new0(char, strlen(i->argument)+1);
> +        if (!xattr)
> +                return log_oom();
> +
> +        tmp = strv_split(i->argument, WHITESPACE);
> +        if (!tmp)
> +                return log_oom();
> +
> +        for (n = 0; n < strv_length(tmp); ++n) {

strv_length() is relatively expensive (O(n)), please cache the result.


> +                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);

Hmm, this also is a job for split_pair and cunescape, no? Or am I confused?

Otherwise looks fine!

Lennart

-- 
Lennart Poettering - Red Hat, Inc.