From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alan Jenkins Date: Mon, 01 Sep 2008 17:37:11 +0000 Subject: Re: [PATCH] Some function locals in udev_rules_parse.c were needlessly Message-Id: <48BC2847.1050501@tuffmail.co.uk> List-Id: References: <48BAD549.6080304@tuffmail.co.uk> In-Reply-To: <48BAD549.6080304@tuffmail.co.uk> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: linux-hotplug@vger.kernel.org Kay Sievers wrote: > On Sun, Aug 31, 2008 at 19:30, Alan Jenkins wrote: > >> This does not affect current behaviour. However, it is required to >> make the functions thread-safe. (I'm playing with a threaded udevd). >> > > >> - 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