[PATCH] udevd: Avoid implicit memset in match_attr()

[PATCH] udevd: Avoid implicit memset in match_attr()

[Alan Jenkins]

C initializers include an implicit "others => 0" (Ada syntax).
Initializing a char array to "" is equivalent to a memset()
call - which is exactly what it gets compiled to.

Fixing this one callsite reduced memset() _user_ cpu cycles
from 2-4% to 0.05% on the EeePC.

There are other occurences, but the fix isn't that pretty
so I left them alone.

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

diff --git a/udev/udev-rules.c b/udev/udev-rules.c
index 45806c3..ad2b00f 100644
--- a/udev/udev-rules.c
+++ b/udev/udev-rules.c
@@ -1559,10 +1559,11 @@ static int match_attr(struct udev_rules *rules, struct udev_device *dev, struct
 	char attr[UTIL_PATH_SIZE];
 	const char *key_name = &rules->buf[cur->key.attr_off];
 	const char *key_value = &rules->buf[cur->key.value_off];
-	char value[UTIL_NAME_SIZE] = "";
+	char value[UTIL_NAME_SIZE];
 	size_t len;
 
 	util_strlcpy(attr, key_name, sizeof(attr));
+	util_strlcpy(value, "", sizeof(value));
 	util_resolve_subsys_kernel(event->udev, attr, value, sizeof(value), 1);
 	if (value[0] = '\0') {
 		const char *val;

Re: [PATCH] udevd: Avoid implicit memset in match_attr()

[Kay Sievers]

On Sat, Oct 25, 2008 at 16:51, Alan Jenkins <alan-jenkins@tuffmail.co.uk> wrote:
> C initializers include an implicit "others => 0" (Ada syntax).
> Initializing a char array to "" is equivalent to a memset()
> call - which is exactly what it gets compiled to.
>
> Fixing this one callsite reduced memset() _user_ cpu cycles
> from 2-4% to 0.05% on the EeePC.
>
> There are other occurences, but the fix isn't that pretty
> so I left them alone.
>
> Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
>
> diff --git a/udev/udev-rules.c b/udev/udev-rules.c
> index 45806c3..ad2b00f 100644
> --- a/udev/udev-rules.c
> +++ b/udev/udev-rules.c
> @@ -1559,10 +1559,11 @@ static int match_attr(struct udev_rules *rules, struct udev_device *dev, struct
>        char attr[UTIL_PATH_SIZE];
>        const char *key_name = &rules->buf[cur->key.attr_off];
>        const char *key_value = &rules->buf[cur->key.value_off];
> -       char value[UTIL_NAME_SIZE] = "";
> +       char value[UTIL_NAME_SIZE];
>        size_t len;
>
>        util_strlcpy(attr, key_name, sizeof(attr));
> +       util_strlcpy(value, "", sizeof(value));

Wouldn't value[0] = '\0' be even faster? :)

Kay

Re: [PATCH] udevd: Avoid implicit memset in match_attr()

[Alan Jenkins]

Kay Sievers wrote:
> On Sat, Oct 25, 2008 at 16:51, Alan Jenkins <alan-jenkins@tuffmail.co.uk> wrote:
>   
>> C initializers include an implicit "others => 0" (Ada syntax).
>> Initializing a char array to "" is equivalent to a memset()
>> call - which is exactly what it gets compiled to.
>>
>> Fixing this one callsite reduced memset() _user_ cpu cycles
>> from 2-4% to 0.05% on the EeePC.
>>
>> There are other occurences, but the fix isn't that pretty
>> so I left them alone.
>>
>> Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
>>
>> diff --git a/udev/udev-rules.c b/udev/udev-rules.c
>> index 45806c3..ad2b00f 100644
>> --- a/udev/udev-rules.c
>> +++ b/udev/udev-rules.c
>> @@ -1559,10 +1559,11 @@ static int match_attr(struct udev_rules *rules, struct udev_device *dev, struct
>>        char attr[UTIL_PATH_SIZE];
>>        const char *key_name = &rules->buf[cur->key.attr_off];
>>        const char *key_value = &rules->buf[cur->key.value_off];
>> -       char value[UTIL_NAME_SIZE] = "";
>> +       char value[UTIL_NAME_SIZE];
>>        size_t len;
>>
>>        util_strlcpy(attr, key_name, sizeof(attr));
>> +       util_strlcpy(value, "", sizeof(value));
>>     
>
> Wouldn't value[0] = '\0' be even faster? :)
>   

Heh.  That would probably be clearer as well.  There's no big
difference, use whatever you prefer.

Re: [PATCH] udevd: Avoid implicit memset in match_attr()

[Kay Sievers]

On Sat, Oct 25, 2008 at 20:13, Alan Jenkins <alan-jenkins@tuffmail.co.uk> wrote:
> Kay Sievers wrote:
>> On Sat, Oct 25, 2008 at 16:51, Alan Jenkins <alan-jenkins@tuffmail.co.uk> wrote:
>>
>>> C initializers include an implicit "others => 0" (Ada syntax).
>>> Initializing a char array to "" is equivalent to a memset()
>>> call - which is exactly what it gets compiled to.
>>>
>>> Fixing this one callsite reduced memset() _user_ cpu cycles
>>> from 2-4% to 0.05% on the EeePC.
>>>
>>> There are other occurences, but the fix isn't that pretty
>>> so I left them alone.

>>> -       char value[UTIL_NAME_SIZE] = "";
>>> +       char value[UTIL_NAME_SIZE];
>>>        size_t len;
>>>
>>>        util_strlcpy(attr, key_name, sizeof(attr));
>>> +       util_strlcpy(value, "", sizeof(value));
>>
>> Wouldn't value[0] = '\0' be even faster? :)
>
> Heh.  That would probably be clearer as well.  There's no big
> difference, use whatever you prefer.

I applied your patch, and changed all the other occurrences too. The
match_attr() is still pretty high in the profile, so we do one of the
srlcpy() now only if we need to.

Thanks,
Kay