[systemd-devel] [PATCHv2 1/2] udev: export tags of "dead" device nodes to /run/udev/static_node-tags/

[Tom Gundersen]

On Sun, Jul 14, 2013 at 2:51 PM, Dave Reisner <d at falconindy.com> wrote:
>> @@ -152,9 +153,9 @@ enum token_type {
>>          TK_A_OWNER_ID,                  /* uid_t */
>>          TK_A_GROUP_ID,                  /* gid_t */
>>          TK_A_MODE_ID,                   /* mode_t */
>> +        TK_A_TAG,                       /* val */
>>          TK_A_STATIC_NODE,               /* val */
>>          TK_A_ENV,                       /* val, attr */
>> -        TK_A_TAG,                       /* val */
>
> Is this swap really needed?

Yeah, this indicates the precedence of the tokens and we need to
handle the TAG tokens before the STATIC_NODE ones.

>> @@ -2532,18 +2540,56 @@ void udev_rules_apply_static_dev_perms(struct udev_rules *rules)
>>                  case TK_A_MODE_ID:
>>                          mode = cur->key.mode;
>>                          break;
>> +                case TK_A_TAG:
>> +                        t = strv_append(tags, rules_str(rules, cur->key.value_off));
>
> Isn't strv_extend what you want here? It wouldn't copy the whole string
> array, just push the string onto it.

Indeed, fixed.

>> +finish:
>> +        if (f) {
>> +                fflush(f);
>> +                fchmod(fileno(f), 0644);
>> +                if (ferror(f) || rename(path, "/run/udev/static_node-tags") < 0) {
>> +                        r = -errno;
>
> I'm not sure this will capture a useful/accurate errno if fflush()
> fails, setting the error flag on the FILE*.

As discussed on IRC, this should be fine.

>> diff --git a/src/udev/udevd.c b/src/udev/udevd.c
>> index c4127cd..285f9a0 100644
>> --- a/src/udev/udevd.c
>> +++ b/src/udev/udevd.c
>> @@ -1197,7 +1197,8 @@ int main(int argc, char *argv[])
>>          }
>>          log_debug("set children_max to %u\n", children_max);
>>
>> -        udev_rules_apply_static_dev_perms(rules);
>> +        if (udev_rules_apply_static_dev_perms(rules) < 0)
>> +                log_error("failed to apply permissions on static device nodes\n");
>
> Hmm, not going to use the return from udev_rules_apply_static_dev_perms
> to add to the error?

Fixed.

Thanks,

Tom