[PATCH] threadsafe rules iteration

[PATCH] threadsafe rules iteration

[Alan Jenkins]

Move ->current out of struct udev_rules and into a new struct udev_rules_iter.

Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>

diff --git a/udev/udev_rules.c b/udev/udev_rules.c
index da7a62a..c21d762 100644
--- a/udev/udev_rules.c
+++ b/udev/udev_rules.c
@@ -1378,6 +1378,7 @@ nomatch:
 
 int udev_rules_get_name(struct udev_rules *rules, struct udevice *udev)
 {
+	struct udev_rules_iter iter;
 	struct udev_rule *rule;
 	int name_set = 0;
 
@@ -1385,9 +1386,9 @@ int udev_rules_get_name(struct udev_rules *rules, struct udevice *udev)
 	dbg("udev->dev->kernel='%s'\n", udev->dev->kernel);
 
 	/* look for a matching rule to apply */
-	udev_rules_iter_init(rules);
+	udev_rules_iter_init(&iter, rules);
 	while (1) {
-		rule = udev_rules_iter_next(rules);
+		rule = udev_rules_iter_next(&iter);
 		if (rule = NULL)
 			break;
 
@@ -1540,7 +1541,7 @@ int udev_rules_get_name(struct udev_rules *rules, struct udevice *udev)
 
 			if (rule->goto_label.operation != KEY_OP_UNSET) {
 				dbg("moving forward to label '%s'\n", key_val(rule, &rule->goto_label));
-				udev_rules_iter_label(rules, key_val(rule, &rule->goto_label));
+				udev_rules_iter_label(&iter, key_val(rule, &rule->goto_label));
 			}
 		}
 	}
@@ -1561,14 +1562,15 @@ int udev_rules_get_name(struct udev_rules *rules, struct udevice *udev)
 
 int udev_rules_get_run(struct udev_rules *rules, struct udevice *udev)
 {
+	struct udev_rules_iter *iter;
 	struct udev_rule *rule;
 
 	dbg("udev->kernel='%s'\n", udev->dev->kernel);
 
 	/* look for a matching rule to apply */
-	udev_rules_iter_init(rules);
+	udev_rules_iter_init(&iter, rules);
 	while (1) {
-		rule = udev_rules_iter_next(rules);
+		rule = udev_rules_iter_next(&iter);
 		if (rule = NULL)
 			break;
 
@@ -1619,7 +1621,7 @@ int udev_rules_get_run(struct udev_rules *rules, struct udevice *udev)
 
 			if (rule->goto_label.operation != KEY_OP_UNSET) {
 				dbg("moving forward to label '%s'\n", key_val(rule, &rule->goto_label));
-				udev_rules_iter_label(rules, key_val(rule, &rule->goto_label));
+				udev_rules_iter_label(&iter, key_val(rule, &rule->goto_label));
 			}
 		}
 	}
diff --git a/udev/udev_rules.h b/udev/udev_rules.h
index 9a41ccb..fd82f5d 100644
--- a/udev/udev_rules.h
+++ b/udev/udev_rules.h
@@ -110,16 +110,20 @@ struct udev_rule {
 struct udev_rules {
 	char *buf;
 	size_t bufsize;
-	size_t current;
 	int resolve_names;
 };
 
+struct udev_rules_iter {
+	struct udev_rules *rules;
+	size_t current;
+};
+
 extern int udev_rules_init(struct udev_rules *rules, int resolve_names);
 extern void udev_rules_cleanup(struct udev_rules *rules);
 
-extern void udev_rules_iter_init(struct udev_rules *rules);
-extern struct udev_rule *udev_rules_iter_next(struct udev_rules *rules);
-extern struct udev_rule *udev_rules_iter_label(struct udev_rules *rules, const char *label);
+extern void udev_rules_iter_init(struct udev_rules_iter *iter, struct udev_rules *rules);
+extern struct udev_rule *udev_rules_iter_next(struct udev_rules_iter *iter);
+extern struct udev_rule *udev_rules_iter_label(struct udev_rules_iter *iter, const char *label);
 
 extern int udev_rules_get_name(struct udev_rules *rules, struct udevice *udev);
 extern int udev_rules_get_run(struct udev_rules *rules, struct udevice *udev);
diff --git a/udev/udev_rules_parse.c b/udev/udev_rules_parse.c
index 90b139b..a9a483c 100644
--- a/udev/udev_rules_parse.c
+++ b/udev/udev_rules_parse.c
@@ -33,49 +33,53 @@
 #include "udev_selinux.h"
 
 
-void udev_rules_iter_init(struct udev_rules *rules)
+void udev_rules_iter_init(struct udev_rules_iter *iter, struct udev_rules *rules)
 {
 	dbg("bufsize=%zi\n", rules->bufsize);
-	rules->current = 0;
+	iter->rules = rules;
+	iter->current = 0;
 }
 
-struct udev_rule *udev_rules_iter_next(struct udev_rules *rules)
+struct udev_rule *udev_rules_iter_next(struct udev_rules_iter *iter)
 {
+	struct udev_rules *rules;
 	struct udev_rule *rule;
 
+	rules = iter->rules;
 	if (!rules)
 		return NULL;
 
-	dbg("current=%zi\n", rules->current);
-	if (rules->current >= rules->bufsize) {
+	dbg("current=%zi\n", iter->current);
+	if (iter->current >= rules->bufsize) {
 		dbg("no more rules\n");
 		return NULL;
 	}
 
 	/* get next rule */
-	rule = (struct udev_rule *) (rules->buf + rules->current);
-	rules->current += sizeof(struct udev_rule) + rule->bufsize;
+	rule = (struct udev_rule *) (rules->buf + iter->current);
+	iter->current += sizeof(struct udev_rule) + rule->bufsize;
 
 	return rule;
 }
 
-struct udev_rule *udev_rules_iter_label(struct udev_rules *rules, const char *label)
+struct udev_rule *udev_rules_iter_label(struct udev_rules_iter *iter, const char *label)
 {
 	struct udev_rule *rule;
-	size_t start = rules->current;
-
+	struct udev_rules *rules = iter->rules;
+	size_t start = iter->current;
+	
 next:
-	dbg("current=%zi\n", rules->current);
-	if (rules->current >= rules->bufsize) {
+	dbg("current=%zi\n", iter->current);
+	if (iter->current >= rules->bufsize) {
 		err("LABEL='%s' not found, GOTO will be ignored\n", label);
-		rules->current = start;
+		iter->current = start;
 		return NULL;
 	}
-	rule = (struct udev_rule *) (rules->buf + rules->current);
+	rule = (struct udev_rule *) (rules->buf + iter->current);
 
 	if (strcmp(&rule->buf[rule->label.val_off], label) != 0) {
 		dbg("moving forward, looking for label '%s'\n", label);
-		rules->current += sizeof(struct udev_rule) + rule->bufsize;
+		iter->current += sizeof(struct udev_rule) + rule->bufsize;
 		goto next;
 	}
 

Re: [PATCH] threadsafe rules iteration

[Alan Jenkins]

Alan Jenkins wrote:
> Move ->current out of struct udev_rules and into a new struct udev_rules_iter.
>
> Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
>
> diff --git a/udev/udev_rules.c b/udev/udev_rules.c
> index da7a62a..c21d762 100644
> --- a/udev/udev_rules.c
> +++ b/udev/udev_rules.c
>   

> @@ -1561,14 +1562,15 @@ int udev_rules_get_name(struct udev_rules *rules, struct udevice *udev)
>  
>  int udev_rules_get_run(struct udev_rules *rules, struct udevice *udev)
>  {
> +	struct udev_rules_iter *iter;
>   

Sorry, don't apply that.  It should be

	struct udev_rules_iter iter;


I think we should be building with -Werror, but I'm not sure how to tell
autoconf/automake.

udev_rules.c: In function ‘udev_rules_get_run’:
udev_rules.c:1571: warning: passing argument 1 of ‘udev_rules_iter_init’
from incompatible pointer type
udev_rules.c:1573: warning: passing argument 1 of ‘udev_rules_iter_next’
from incompatible pointer type
udev_rules.c:1624: warning: passing argument 1 of
‘udev_rules_iter_label’ from incompatible pointer type

Re: [PATCH] threadsafe rules iteration

[Scott James Remnant]

[-- Attachment #1: Type: text/plain, Size: 232 bytes --]

On Fri, 2008-09-05 at 16:08 +0100, Alan Jenkins wrote:

> I think we should be building with -Werror, but I'm not sure how to tell
> autoconf/automake.
> 
CFLAGS=-Werror

Scott
-- 
Scott James Remnant
scott@canonical.com

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] threadsafe rules iteration

[Alan Jenkins]

Scott James Remnant wrote:
> On Fri, 2008-09-05 at 16:08 +0100, Alan Jenkins wrote:
>
>   
>> I think we should be building with -Werror, but I'm not sure how to tell
>> autoconf/automake.
>>
>>     
> CFLAGS=-Werror
>
> Scott
>   
That doesn't seem to work if I add it to configure.ac...

        compiler:               /home/alan/bin/cc
        cflags:                 -g -O2
        ldflags:

I was hoping for something I could e.g. submit as a patch.

Alan

Re: [PATCH] threadsafe rules iteration

[Kay Sievers]

On Mon, Sep 8, 2008 at 12:45, Alan Jenkins <alan-jenkins@tuffmail.co.uk> wrote:
> Scott James Remnant wrote:
>> On Fri, 2008-09-05 at 16:08 +0100, Alan Jenkins wrote:
>>
>>
>>> I think we should be building with -Werror, but I'm not sure how to tell
>>> autoconf/automake.
>>>
>>>
>> CFLAGS=-Werror
>>
>> Scott
>>
> That doesn't seem to work if I add it to configure.ac...
>
>        compiler:               /home/alan/bin/cc
>        cflags:                 -g -O2
>        ldflags:
>
> I was hoping for something I could e.g. submit as a patch.

Look at autogen.sh, where the options/warnings are listed I use by
default, but which are not distributed. We better not make it the
default, you can just pass it to your own configure line.

Kay

Re: [PATCH] threadsafe rules iteration

[Alan Jenkins]

Kay Sievers wrote:
> On Mon, Sep 8, 2008 at 12:45, Alan Jenkins <alan-jenkins@tuffmail.co.uk> wrote:
>   
>> Scott James Remnant wrote:
>>     
>>> On Fri, 2008-09-05 at 16:08 +0100, Alan Jenkins wrote:
>>>
>>>
>>>       
>>>> I think we should be building with -Werror, but I'm not sure how to tell
>>>> autoconf/automake.
>>>>
>>>>
>>>>         
>>> CFLAGS=-Werror
>>>
>>> Scott
>>>
>>>       
>> That doesn't seem to work if I add it to configure.ac...
>>
>>        compiler:               /home/alan/bin/cc
>>        cflags:                 -g -O2
>>        ldflags:
>>
>> I was hoping for something I could e.g. submit as a patch.
>>     
>
> Look at autogen.sh, where the options/warnings are listed I use by
> default, but which are not distributed. We better not make it the
> default, you can just pass it to your own configure line.
>
> Kay
>   

Thanks, I'll do that then.

Alan

Re: [PATCH] threadsafe rules iteration

[Kay Sievers]

On Fri, Sep 5, 2008 at 17:08, Alan Jenkins <alan-jenkins@tuffmail.co.uk> wrote:
> Alan Jenkins wrote:
>> Move ->current out of struct udev_rules and into a new struct udev_rules_iter.

Applied.

Thanks,
Kay