Skip to contentSkip to conversation

qdev-property: Make bit property parsing stricter

PATCH Applied Jan Kiszka 4 participants 5 messages Jan 21, 2012 – Jan 27, 2012

lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

PATCH 1/1 qdev-property: Make bit property parsing stricter

By using strncasecmp, we allow for arbitrary characters after the "on"/"off" string. Fix this by switching to strcasecmp.

Signed-off-by Jan Kiszka <[email protected]>
---
 hw/qdev-properties.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
hw/qdev-properties.c +2 -2
index 02f0dae..ea3b2df 100644
@@ -40,9 +40,9 @@ static void qdev_prop_cpy(DeviceState *dev, Property *props, void *src)
 /* Bit */
 static int parse_bit(DeviceState *dev, Property *prop, const char *str)
 {
-    if (!strncasecmp(str, "on", 2))
+    if (!strcasecmp(str, "on"))
         bit_prop_set(dev, prop, true);
-    else if (!strncasecmp(str, "off", 3))
+    else if (!strcasecmp(str, "off"))
         bit_prop_set(dev, prop, false);
     else
         return -EINVAL;
Signature · 1 line
1.7.3.4
Reviewed-by on v1 1/1
#1 lore raw

Am 21.01.2012 14:43, schrieb Jan Kiszka:

Quoted 4 lines

By using strncasecmp, we allow for arbitrary characters after the "on"/"off" string. Fix this by switching to strcasecmp.

Signed-off-by Jan Kiszka <[email protected]>
Reviewed-by Andreas Färber <[email protected]>

An alternative might be to increase the char count by one. For a const char* parameter I see the responsibility for nul-terminating at the caller though, so this seems right. Did you check all callers?

Andreas

Quoted 20 lines
---
 hw/qdev-properties.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
hw/qdev-properties.c +2 -2
index 02f0dae..ea3b2df 100644
@@ -40,9 +40,9 @@ static void qdev_prop_cpy(DeviceState *dev, Property *props, void *src)
 /* Bit */
 static int parse_bit(DeviceState *dev, Property *prop, const char *str)
 {
-    if (!strncasecmp(str, "on", 2))
+    if (!strcasecmp(str, "on"))
         bit_prop_set(dev, prop, true);
-    else if (!strncasecmp(str, "off", 3))
+    else if (!strcasecmp(str, "off"))
         bit_prop_set(dev, prop, false);
     else
         return -EINVAL;
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
author
#2 lore raw
Quoted 11 lines · Andreas Färber

Am 21.01.2012 14:43, schrieb Jan Kiszka:

Quoted 4 lines

By using strncasecmp, we allow for arbitrary characters after the "on"/"off" string. Fix this by switching to strcasecmp.

Signed-off-by Jan Kiszka <[email protected]>
Reviewed-by Andreas Färber <[email protected]>

An alternative might be to increase the char count by one. For a const char* parameter I see the responsibility for nul-terminating at the caller though, so this seems right. Did you check all callers?

For any normal function that takes a char * string without a maximum length, the responsibility for terminating is on caller side. Also because we checks against patterns of different length. So we don't need to jump through hoops here.

Jan

Reviewed-by
#3 lore raw
Quoted 9 lines · Jan Kiszka
Quoted 8 lines · Andreas Färber

Am 21.01.2012 14:43, schrieb Jan Kiszka:

Quoted 4 lines

By using strncasecmp, we allow for arbitrary characters after the "on"/"off" string. Fix this by switching to strcasecmp.

Signed-off-by Jan Kiszka <[email protected]>
Reviewed-by Andreas Färber <[email protected]>
Quoted 8 lines
Quoted 3 lines

An alternative might be to increase the char count by one. For a const char* parameter I see the responsibility for nul-terminating at the caller though, so this seems right. Did you check all callers?

For any normal function that takes a char * string without a maximum length, the responsibility for terminating is on caller side. Also because we checks against patterns of different length. So we don't need to jump through hoops here.

I agree.

Reviewed-by Markus Armbruster <[email protected]>