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

[Maciej Wereski]

10.11.2014 at 23:55 Lennart Poettering <lennart at poettering.net> wrote:

> On Thu, 30.10.14 12:21, Maciej Wereski (m.wereski at partner.samsung.com)  
> wrote:
>
>>
>> +static int get_xattrs_from_arg(Item *i) {
>> +        _cleanup_free_ char *xattr = NULL;
>> +        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)) > 0) {
>> +                if (strv_extend(&i->xattrs, xattr) < 0)
>> +                        return log_oom();
>> +                free(xattr);
>> +                xattr = NULL;
>> +        }
>
> Please use strv_consume() or strv_push() here, to make the additional
> copy unnecessary. Also please, generate a parse failure if
> unquote_first_workd() fails due to parse errors.

What does it mean parse failure? I'm passing return value of  
unquote_first_word(). Should it be something else?

>> +        r = get_xattrs_from_arg(i);
>> +        if (r < 0)
>> +                return r;
>> +
>> +        if (strv_isempty(i->xattrs))
>> +                return 0;
>> +
>> +        STRV_FOREACH(x, i->xattrs) {
>> +                _cleanup_free_ char *name = NULL, *value = NULL, *tmp  
>> = NULL;
>> +                n = split_pair(*x, "=", &name, &value);
>> +                if (n < 0)
>> +                        return n;
>> +                tmp = unquote(value, "\"");
>> +                if (!tmp)
>> +                        return log_oom();
>> +                free(value);
>> +                value = cunescape(tmp);
>> +                if (!value)
>> +                        return log_oom();
>> +                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;
>> +                }
>
> The indentation is wrong.
> [cut]

Hmm? I'm using systemd .vimrc, so what is wrong with indentation here  
precisely?

>>
>> @@ -894,6 +983,12 @@ static int create_item(Item *i) {
>>                  r = glob_item(i, item_set_perms);
>>                  if (r < 0)
>>                          return r;
>> +
>> +                if (i->xattrs) {
>> +                        r = glob_item(i, item_set_xattrs);
>> +                        if (r < 0)
>> +                                return r;
>> +                }
>>                  break;
>
> Hmm, ths would mean we resolve the globbing twice. Once for
> item_set_perms() and once for item_set_xattr(). I think it would be
> better to do this in one call.
>

I've removed this completely. item_set_perms already calls label_fix,  
which calls SMACK function.

regards,
-- 
Maciej Wereski
Samsung R&D Institute Poland
Samsung Electronics
m.wereski at partner.samsung.com