Re: [RFC PATCH] Split udev_sysfs.c to leave out caches from libudev

[Alan Jenkins <alan-jenkins@tuffmail.co.uk>]

Alan Jenkins wrote:
> Kay Sievers wrote:
>   
>> On Tue, Sep 2, 2008 at 13:09, Alan Jenkins <alan-jenkins@tuffmail.co.uk> wrote:
>>   
>>     
>>> udev_sysfs.c device and attribute value caches are only appropriate for
>>> short-lived processes, like udev-event and usbid.  In long lived processes
>>> they would fill up with stale data.
>>>     
>>>       
>> Sure, the cache was never coded to be used in threaded programs.
>>
>>   
>>     
>>> Fortunately libudev doesn't use these caches.
>>>     
>>>       
>> For good reason, yeah. And it will not, at least not in its current state.
>>
>>   
>>     
>>> But this should be made
>>> explicit to guard against future changes.  Let's move the caches into a
>>> separate file, with its own initialization and cleanup functions.
>>>     
>>>       
>> Wouldn't it be nicer to create a sysfs_cache object, which is passed
>> to all these functions? The "cheap" tools can just use NULL there, but
>> any possible user could have more than one instance of the cache. The
>> cache is really needed if you do thousand of rule compares against a
>> sysfs value, over and over. I guess we should start changing the stuff
>> to work with local vars, instead of moving the current global stuff to
>> a different file?
>>   
>>     
>
> Good point.  That helps make the cache usage more explicit, and it's
> much nicer for my purposes as well.  (I had blindly converted the global
> cache variables to per-thread variables, without touching the callers).
>
> Ok.  To keep the existing behaviour where udev_rules_get_run() reuses
> the cache from udev_rules_get_name(), they both need a sysfs_cache
> parameter.  Then I have to change test-udev and udevtest as well as
> udevd - no point having test programs if they use different code paths. 
> I'll see what that looks like then and resend.
>   
Oops.  sysfs_cache can't be made optional ("just use NULL") without a
bit more effort.  At the moment we rely on sysfs_cleanup() freeing
everything in the cache.  If the cache isn't used, we need a new API to
free individual results.

So we also need sysfs_device_cleanup(), to free the device if it's not
on a cache list.  And sysfs_attr_get_value() can copy the result to a
buffer instead of returning a string.  I think that's reasonable.

Alan