Re: [PATCH] libudev: Allocate udev_device->envp lazily

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

Bob Beers wrote:
> one small question ...
>
> On Tue, Oct 21, 2008 at 6:11 AM, Alan Jenkins
> <alan-jenkins@tuffmail.co.uk> wrote:
>   
>> Allocate udev_device->envp lazily
>>
>> It's a pity to allocate 128 words when we don't need them.
>> Measured 2% _user_ cpu time reduction on EeePC coldplug.
>>
>> Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
>>
>> diff --git a/udev/lib/libudev-device.c b/udev/lib/libudev-device.c
>> index b54c727..69ab1f7 100644
>> --- a/udev/lib/libudev-device.c
>> +++ b/udev/lib/libudev-device.c
>> @@ -31,6 +31,8 @@
>>  #include "libudev.h"
>>  #include "libudev-private.h"
>>
>> +#define ENVP_SIZE 128
>> +
>>  struct udev_device {
>>        int refcount;
>>        struct udev *udev;
>> @@ -46,7 +48,7 @@ struct udev_device {
>>        struct udev_list_node devlinks_list;
>>        int devlinks_uptodate;
>>        struct udev_list_node properties_list;
>> -       char *envp[128];
>> +       char **envp;
>>        int envp_uptodate;
>>        char *driver;
>>        int driver_set;
>> @@ -611,8 +613,11 @@ void udev_device_unref(struct udev_device *udev_device)
>>        free(udev_device->devpath_old);
>>        free(udev_device->physdevpath);
>>        udev_list_cleanup_entries(udev_device->udev, &udev_device->sysattr_list);
>> -       for (i = 0; i < ARRAY_SIZE(udev_device->envp) && udev_device->envp[i] != NULL; i++)
>> -               free(udev_device->envp[i]);
>> +       if (udev_device->envp != NULL) {
>> +               for (i = 0; i < ENVP_SIZE && udev_device->envp[i] != NULL; i++)
>> +                       free(udev_device->envp[i]);
>> +               free(udev_device->envp);
>> +       }
>>        info(udev_device->udev, "udev_device: %p released\n", udev_device);
>>        free(udev_device);
>>  }
>> @@ -1014,14 +1019,18 @@ char **udev_device_get_properties_envp(struct udev_device *udev_device)
>>                unsigned int i;
>>                struct udev_list_entry *list_entry;
>>
>> -               for (i = 0; i < ARRAY_SIZE(udev_device->envp) && udev_device->envp[i] != NULL; i++)
>> -                       free(udev_device->envp[i]);
>> +               if (udev_device->envp) {
>> +                       for (i = 0; i < 128 && udev_device->envp[i] != NULL; i++)
>>     
>
> should this 128 also be ENVP_SIZE?
>   

Yes, sorry.  I see Kay fixed it already though :)

Alan