On Sun, Jul 14, 2013 at 2:51 PM, Dave Reisner <d...@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 _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel