Skip to contentSkip to conversation

Some function locals in udev_rules_parse.c were needlessly

PATCH Applied Alan Jenkins 2 participants 4 messages Aug 31, 2008 – Sep 1, 2008

lore
Share & tools
lore

Alan Jenkins

author
#0 lore raw

PATCH 1/1 Some function locals in udev_rules_parse.c were needlessly

This does not affect current behaviour. However, it is required to make the functions thread-safe. (I'm playing with a threaded udevd).

Signed-off-by Alan Jenkins <[email protected]>
udev/udev_rules_parse.c +2 -2
index 705b3fe..90b139b 100644
@@ -41,7 +41,7 @@ void udev_rules_iter_init(struct udev_rules *rules)
 
 struct udev_rule *udev_rules_iter_next(struct udev_rules *rules)
 {
-	static struct udev_rule *rule;
+	struct udev_rule *rule;
 
 	if (!rules)
 		return NULL;

@@ -61,7 +61,7 @@ struct udev_rule *udev_rules_iter_next(struct udev_rules *rules)
 
 struct udev_rule *udev_rules_iter_label(struct udev_rules *rules, const char *label)
 {
-	static struct udev_rule *rule;
+	struct udev_rule *rule;
 	size_t start = rules->current;
 
 next:
#1 lore raw

Re: [PATCH] Some function locals in udev_rules_parse.c were needlessly declared as static

Review

Quoted 5 lines · Alan Jenkins

This does not affect current behaviour. However, it is required to make the functions thread-safe. (I'm playing with a threaded udevd).

(quoted diff) +1 -1
-       static struct udev_rule *rule;
+       struct udev_rule *rule;

It's not needlessly static, we return _this_ value. The parsing stuff is not thread safe at the moment, we would need a real fix, this would break it.

Thanks,
Kay

author
#2 lore raw
In reply to Alan Jenkins
Quoted 7 lines · Kay Sievers
Quoted 3 lines · Alan Jenkins

This does not affect current behaviour. However, it is required to make the functions thread-safe. (I'm playing with a threaded udevd).

Quoted 8 lines

Review

Quoted 3 lines
(quoted diff) +1 -1
-       static struct udev_rule *rule;
+       struct udev_rule *rule;

It's not needlessly static, we return _this_ value. The parsing stuff is not thread safe at the moment, we would need a real fix, this would break it.

Thanks for looking at these patches.

The functions do "return rule", but they always write to it before reading it, so there's no persistent state here. And they don't "return &rule", so it's fine for the variable to be on the stack. Did I miss something?

I belatedly noticed the other parsing stuff a few hours after posting the patch :-). I hacked it up and (with unpublished patches) finally got a threaded udevd which appeared to work. (I also did per-thread environment variable emulation, and fixed the caches in udev_sysfs.c for thread-safety). So empirically I had judged this patch correct. And my unpublished patches wouldn't conflict with or obsolete this one.

Regards
Alan

Applied
#3 lore raw

Re: [PATCH] Some function locals in udev_rules_parse.c were

In reply to Alan Jenkins
Quoted 16 lines · Alan Jenkins
Quoted 7 lines · Kay Sievers
Quoted 3 lines · Alan Jenkins

This does not affect current behaviour. However, it is required to make the functions thread-safe. (I'm playing with a threaded udevd).

Quoted 8 lines

Review

Quoted 3 lines
(quoted diff) +1 -1
-       static struct udev_rule *rule;
+       struct udev_rule *rule;

It's not needlessly static, we return _this_ value. The parsing stuff is not thread safe at the moment, we would need a real fix, this would break it.

Quoted 6 lines

Thanks for looking at these patches.

The functions do "return rule", but they always write to it before reading it, so there's no persistent state here. And they don't "return &rule", so it's fine for the variable to be on the stack. Did I miss something?

Ah, they should return a pointer to the data passed in to the function, which should be fine.

Applied.

Thanks,
Kay