Skip to contentSkip to conversation

udevd: Avoid implicit memset in match_attr()

PATCH In review Alan Jenkins 2 participants 4 messages Oct 25, 2008 – Oct 26, 2008

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 udevd: Avoid implicit memset in match_attr()

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.

udev/udev-rules.c +2 -1
index 45806c3..ad2b00f 100644
@@ -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;
#1 lore raw
Quoted 17 lines · 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.

udev/udev-rules.c +0 -0
index 45806c3..ad2b00f 100644

udev/udev-rules.c static int match_attr(struct udev_rules *rules, struct udev_device *dev, struct · line 1566

Quoted 9 lines
udev/udev-rules.c +2 -1
@@ -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

#2 lore raw
In reply to Alan Jenkins
Quoted 26 lines · Kay Sievers
Quoted 24 lines · 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.

udev/udev-rules.c +1 -1
index 45806c3..ad2b00f 100644
@@ -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;
 
Quoted 6 lines

udev/udev-rules.c static int match_attr(struct udev_rules *rules, struct udev_device *dev, struct

Quoted 3 lines
udev/udev-rules.c +1 -0
       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.

#3 lore raw
In reply to Alan Jenkins
Quoted 17 lines · Alan Jenkins
Quoted 16 lines · Kay Sievers
Quoted 14 lines · 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.

udev/udev-rules.c +1 -1
-       char value[UTIL_NAME_SIZE] = "";
+       char value[UTIL_NAME_SIZE];
       size_t len;
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
       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