diff mbox series

[1/9] hw/core/qdev-properties: Use qemu_strtol() in set_mac() handler

Message ID 20200313184607.11792-2-philmd@redhat.com
State New
Headers show
Series user-mode: Prune build dependencies (part 3) | expand

Commit Message

Philippe Mathieu-Daudé March 13, 2020, 6:45 p.m. UTC 
Replace strtol() by qemu_strtol() so checkpatch.pl won't complain
if we move this code later.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/core/qdev-properties.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Richard Henderson March 15, 2020, 9:25 p.m. UTC | #1 
On 3/13/20 11:45 AM, Philippe Mathieu-Daudé wrote:
> +        if (qemu_strtol(str + pos, &p, 16, &val) < 0 || val > 0xff) {
> +            goto inval;
> +        }

This is doing more that *just* using qemu_strtol, it's also validating the
input.  I don't think you need to adjust the patch, just improve the commit
message.  With that,

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~
Philippe Mathieu-Daudé March 15, 2020, 10:28 p.m. UTC | #2 
On 3/15/20 10:25 PM, Richard Henderson wrote:
> On 3/13/20 11:45 AM, Philippe Mathieu-Daudé wrote:
>> +        if (qemu_strtol(str + pos, &p, 16, &val) < 0 || val > 0xff) {
>> +            goto inval;
>> +        }
> 
> This is doing more that *just* using qemu_strtol, it's also validating the
> input.  I don't think you need to adjust the patch, just improve the commit
> message.  With that,
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Correct. I'll add a comment, as currently we ignore incorrect input due 
to the implicit cast to uint8_t:

struct MACAddr {
     uint8_t a[6];
};

     mac->a[i] = strtol(str+pos, &p, 16);

Thanks!

> 
> r~
>
diff mbox series

Patch

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 59380ed761..48e4c98cf0 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1,4 +1,5 @@ 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "net/net.h"
 #include "hw/qdev-properties.h"
 #include "qapi/error.h"
@@ -532,7 +533,8 @@  static void set_mac(Object *obj, Visitor *v, const char *name, void *opaque,
     MACAddr *mac = qdev_get_prop_ptr(dev, prop);
     Error *local_err = NULL;
     int i, pos;
-    char *str, *p;
+    char *str;
+    const char *p;
 
     if (dev->realized) {
         qdev_prop_set_after_realize(dev, name, errp);
@@ -546,6 +548,8 @@  static void set_mac(Object *obj, Visitor *v, const char *name, void *opaque,
     }
 
     for (i = 0, pos = 0; i < 6; i++, pos += 3) {
+        long val;
+
         if (!qemu_isxdigit(str[pos])) {
             goto inval;
         }
@@ -561,7 +565,10 @@  static void set_mac(Object *obj, Visitor *v, const char *name, void *opaque,
                 goto inval;
             }
         }
-        mac->a[i] = strtol(str+pos, &p, 16);
+        if (qemu_strtol(str + pos, &p, 16, &val) < 0 || val > 0xff) {
+            goto inval;
+        }
+        mac->a[i] = val;
     }
     g_free(str);
     return;