From 09f1bbcd834c63da35b3fe190fcd9a20d7b04f07 Mon Sep 17 00:00:00 2001 From: Michael Roth Date: Sun, 4 Mar 2012 13:38:27 -0600 Subject: qdev: Use int32_t container for devfn property MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Valid range for devfn is -1 to 255 (-1 for automatic assignment). We do not currently validate this due to devfn being stored as a uint32_t. This can lead to segfaults and other strange behavior. We could technically just cast it to int32_t to implement the checking, but this will not work for visitor-based setting where we may do additional bounds-checking based on target container type, which is int32_t for this case. Signed-off-by: Michael Roth Signed-off-by: Paolo Bonzini Signed-off-by: Andreas Färber --- hw/qdev-properties.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) (limited to 'hw/qdev-properties.c') diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index b7b5597..d109f89 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -824,7 +824,7 @@ static void set_pci_devfn(Object *obj, Visitor *v, void *opaque, { DeviceState *dev = DEVICE(obj); Property *prop = opaque; - uint32_t *ptr = qdev_get_prop_ptr(dev, prop); + int32_t *ptr = qdev_get_prop_ptr(dev, prop); unsigned int slot, fn, n; Error *local_err = NULL; char *str; @@ -860,7 +860,7 @@ invalid: static int print_pci_devfn(DeviceState *dev, Property *prop, char *dest, size_t len) { - uint32_t *ptr = qdev_get_prop_ptr(dev, prop); + int32_t *ptr = qdev_get_prop_ptr(dev, prop); if (*ptr == -1) { return snprintf(dest, len, ""); @@ -875,11 +875,8 @@ PropertyInfo qdev_prop_pci_devfn = { .print = print_pci_devfn, .get = get_int32, .set = set_pci_devfn, - /* FIXME: this should be -1...255, but the address is stored - * into an uint32_t rather than int32_t. - */ - .min = 0, - .max = 0xFFFFFFFFULL, + .min = -1, + .max = 255, }; /* --- blocksize --- */ -- cgit v1.1 From c08fb2ac000b636a8426e69d93f7340274d76c9e Mon Sep 17 00:00:00 2001 From: Michael Roth Date: Wed, 22 Feb 2012 12:26:37 -0600 Subject: qdev: Switch property accessors to fixed-width visitor interfaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This introduces {get,set}_uint{8,16,32,64}() functions for the respective qdev types. TADDR and VLAN are switched to explicit int64, BLOCKSIZE to uint16. Signed-off-by: Michael Roth Signed-off-by: Paolo Bonzini Signed-off-by: Andreas Färber --- hw/qdev-properties.c | 150 ++++++++++++++++++++++++++++++--------------------- 1 file changed, 89 insertions(+), 61 deletions(-) (limited to 'hw/qdev-properties.c') diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index d109f89..553b0d1 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -76,33 +76,30 @@ PropertyInfo qdev_prop_bit = { /* --- 8bit integer --- */ -static void get_int8(Object *obj, Visitor *v, void *opaque, - const char *name, Error **errp) +static void get_uint8(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) { DeviceState *dev = DEVICE(obj); Property *prop = opaque; - int8_t *ptr = qdev_get_prop_ptr(dev, prop); - int64_t value; + uint8_t *ptr = qdev_get_prop_ptr(dev, prop); - value = *ptr; - visit_type_int(v, &value, name, errp); + visit_type_uint8(v, ptr, name, errp); } -static void set_int8(Object *obj, Visitor *v, void *opaque, - const char *name, Error **errp) +static void set_uint8(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) { DeviceState *dev = DEVICE(obj); Property *prop = opaque; - int8_t *ptr = qdev_get_prop_ptr(dev, prop); + uint8_t value, *ptr = qdev_get_prop_ptr(dev, prop); Error *local_err = NULL; - int64_t value; if (dev->state != DEV_STATE_CREATED) { error_set(errp, QERR_PERMISSION_DENIED); return; } - visit_type_int(v, &value, name, &local_err); + visit_type_uint8(v, &value, name, &local_err); if (local_err) { error_propagate(errp, local_err); return; @@ -111,15 +108,15 @@ static void set_int8(Object *obj, Visitor *v, void *opaque, *ptr = value; } else { error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, - dev->id?:"", name, value, prop->info->min, + dev->id?:"", name, (int64_t)value, prop->info->min, prop->info->max); } } PropertyInfo qdev_prop_uint8 = { .name = "uint8", - .get = get_int8, - .set = set_int8, + .get = get_uint8, + .set = set_uint8, .min = 0, .max = 255, }; @@ -154,41 +151,38 @@ PropertyInfo qdev_prop_hex8 = { .legacy_name = "hex8", .parse = parse_hex8, .print = print_hex8, - .get = get_int8, - .set = set_int8, + .get = get_uint8, + .set = set_uint8, .min = 0, .max = 255, }; /* --- 16bit integer --- */ -static void get_int16(Object *obj, Visitor *v, void *opaque, - const char *name, Error **errp) +static void get_uint16(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) { DeviceState *dev = DEVICE(obj); Property *prop = opaque; - int16_t *ptr = qdev_get_prop_ptr(dev, prop); - int64_t value; + uint16_t *ptr = qdev_get_prop_ptr(dev, prop); - value = *ptr; - visit_type_int(v, &value, name, errp); + visit_type_uint16(v, ptr, name, errp); } -static void set_int16(Object *obj, Visitor *v, void *opaque, - const char *name, Error **errp) +static void set_uint16(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) { DeviceState *dev = DEVICE(obj); Property *prop = opaque; - int16_t *ptr = qdev_get_prop_ptr(dev, prop); + uint16_t value, *ptr = qdev_get_prop_ptr(dev, prop); Error *local_err = NULL; - int64_t value; if (dev->state != DEV_STATE_CREATED) { error_set(errp, QERR_PERMISSION_DENIED); return; } - visit_type_int(v, &value, name, &local_err); + visit_type_uint16(v, &value, name, &local_err); if (local_err) { error_propagate(errp, local_err); return; @@ -197,31 +191,67 @@ static void set_int16(Object *obj, Visitor *v, void *opaque, *ptr = value; } else { error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, - dev->id?:"", name, value, prop->info->min, + dev->id?:"", name, (int64_t)value, prop->info->min, prop->info->max); } } PropertyInfo qdev_prop_uint16 = { .name = "uint16", - .get = get_int16, - .set = set_int16, + .get = get_uint16, + .set = set_uint16, .min = 0, .max = 65535, }; /* --- 32bit integer --- */ +static void get_uint32(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + DeviceState *dev = DEVICE(obj); + Property *prop = opaque; + uint32_t value, *ptr = qdev_get_prop_ptr(dev, prop); + + value = *ptr; + visit_type_uint32(v, &value, name, errp); +} + +static void set_uint32(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + DeviceState *dev = DEVICE(obj); + Property *prop = opaque; + uint32_t value, *ptr = qdev_get_prop_ptr(dev, prop); + Error *local_err = NULL; + + if (dev->state != DEV_STATE_CREATED) { + error_set(errp, QERR_PERMISSION_DENIED); + return; + } + + visit_type_uint32(v, &value, name, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + if (value >= prop->info->min && value <= prop->info->max) { + *ptr = value; + } else { + error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, + dev->id?:"", name, (int64_t)value, prop->info->min, + prop->info->max); + } +} + static void get_int32(Object *obj, Visitor *v, void *opaque, const char *name, Error **errp) { DeviceState *dev = DEVICE(obj); Property *prop = opaque; int32_t *ptr = qdev_get_prop_ptr(dev, prop); - int64_t value; - value = *ptr; - visit_type_int(v, &value, name, errp); + visit_type_int32(v, ptr, name, errp); } static void set_int32(Object *obj, Visitor *v, void *opaque, @@ -229,16 +259,15 @@ static void set_int32(Object *obj, Visitor *v, void *opaque, { DeviceState *dev = DEVICE(obj); Property *prop = opaque; - int32_t *ptr = qdev_get_prop_ptr(dev, prop); + int32_t value, *ptr = qdev_get_prop_ptr(dev, prop); Error *local_err = NULL; - int64_t value; if (dev->state != DEV_STATE_CREATED) { error_set(errp, QERR_PERMISSION_DENIED); return; } - visit_type_int(v, &value, name, &local_err); + visit_type_int32(v, &value, name, &local_err); if (local_err) { error_propagate(errp, local_err); return; @@ -247,15 +276,15 @@ static void set_int32(Object *obj, Visitor *v, void *opaque, *ptr = value; } else { error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, - dev->id?:"", name, value, prop->info->min, + dev->id?:"", name, (int64_t)value, prop->info->min, prop->info->max); } } PropertyInfo qdev_prop_uint32 = { .name = "uint32", - .get = get_int32, - .set = set_int32, + .get = get_uint32, + .set = set_uint32, .min = 0, .max = 0xFFFFFFFFULL, }; @@ -298,43 +327,43 @@ PropertyInfo qdev_prop_hex32 = { .legacy_name = "hex32", .parse = parse_hex32, .print = print_hex32, - .get = get_int32, - .set = set_int32, + .get = get_uint32, + .set = set_uint32, .min = 0, .max = 0xFFFFFFFFULL, }; /* --- 64bit integer --- */ -static void get_int64(Object *obj, Visitor *v, void *opaque, - const char *name, Error **errp) +static void get_uint64(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) { DeviceState *dev = DEVICE(obj); Property *prop = opaque; - int64_t *ptr = qdev_get_prop_ptr(dev, prop); + uint64_t *ptr = qdev_get_prop_ptr(dev, prop); - visit_type_int(v, ptr, name, errp); + visit_type_uint64(v, ptr, name, errp); } -static void set_int64(Object *obj, Visitor *v, void *opaque, - const char *name, Error **errp) +static void set_uint64(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) { DeviceState *dev = DEVICE(obj); Property *prop = opaque; - int64_t *ptr = qdev_get_prop_ptr(dev, prop); + uint64_t *ptr = qdev_get_prop_ptr(dev, prop); if (dev->state != DEV_STATE_CREATED) { error_set(errp, QERR_PERMISSION_DENIED); return; } - visit_type_int(v, ptr, name, errp); + visit_type_uint64(v, ptr, name, errp); } PropertyInfo qdev_prop_uint64 = { .name = "uint64", - .get = get_int64, - .set = set_int64, + .get = get_uint64, + .set = set_uint64, }; /* --- 64bit hex value --- */ @@ -367,8 +396,8 @@ PropertyInfo qdev_prop_hex64 = { .legacy_name = "hex64", .parse = parse_hex64, .print = print_hex64, - .get = get_int64, - .set = set_int64, + .get = get_uint64, + .set = set_uint64, }; /* --- string --- */ @@ -645,7 +674,7 @@ static void get_vlan(Object *obj, Visitor *v, void *opaque, int64_t id; id = *ptr ? (*ptr)->id : -1; - visit_type_int(v, &id, name, errp); + visit_type_int64(v, &id, name, errp); } static void set_vlan(Object *obj, Visitor *v, void *opaque, @@ -663,7 +692,7 @@ static void set_vlan(Object *obj, Visitor *v, void *opaque, return; } - visit_type_int(v, &id, name, &local_err); + visit_type_int64(v, &id, name, &local_err); if (local_err) { error_propagate(errp, local_err); return; @@ -886,23 +915,22 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque, { DeviceState *dev = DEVICE(obj); Property *prop = opaque; - int16_t *ptr = qdev_get_prop_ptr(dev, prop); + uint16_t value, *ptr = qdev_get_prop_ptr(dev, prop); Error *local_err = NULL; - int64_t value; if (dev->state != DEV_STATE_CREATED) { error_set(errp, QERR_PERMISSION_DENIED); return; } - visit_type_int(v, &value, name, &local_err); + visit_type_uint16(v, &value, name, &local_err); if (local_err) { error_propagate(errp, local_err); return; } if (value < prop->info->min || value > prop->info->max) { error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, - dev->id?:"", name, value, prop->info->min, + dev->id?:"", name, (int64_t)value, prop->info->min, prop->info->max); return; } @@ -910,7 +938,7 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque, /* We rely on power-of-2 blocksizes for bitmasks */ if ((value & (value - 1)) != 0) { error_set(errp, QERR_PROPERTY_VALUE_NOT_POWER_OF_2, - dev->id?:"", name, value); + dev->id?:"", name, (int64_t)value); return; } @@ -919,7 +947,7 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque, PropertyInfo qdev_prop_blocksize = { .name = "blocksize", - .get = get_int16, + .get = get_uint16, .set = set_blocksize, .min = 512, .max = 65024, -- cgit v1.1 From 27712df95d85e76e9d41a0a4ee2acd0ad8a9cc6d Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 12 Apr 2012 15:32:26 +0200 Subject: qdev: Remove PropertyInfo range checking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Range checking in PropertyInfo is now used only for pci_devfn properties and some error reporting. Remove all code that implements it in the various property types, and the now unused fields. Signed-off-by: Paolo Bonzini [AF: Fix blocksize min/max for 32-bit hosts by using const int64_t.] Signed-off-by: Andreas Färber --- hw/qdev-properties.c | 106 +++++++++++++-------------------------------------- 1 file changed, 26 insertions(+), 80 deletions(-) (limited to 'hw/qdev-properties.c') diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 553b0d1..9ae3187 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -91,34 +91,20 @@ static void set_uint8(Object *obj, Visitor *v, void *opaque, { DeviceState *dev = DEVICE(obj); Property *prop = opaque; - uint8_t value, *ptr = qdev_get_prop_ptr(dev, prop); - Error *local_err = NULL; + uint8_t *ptr = qdev_get_prop_ptr(dev, prop); if (dev->state != DEV_STATE_CREATED) { error_set(errp, QERR_PERMISSION_DENIED); return; } - visit_type_uint8(v, &value, name, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } - if (value >= prop->info->min && value <= prop->info->max) { - *ptr = value; - } else { - error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, - dev->id?:"", name, (int64_t)value, prop->info->min, - prop->info->max); - } + visit_type_uint8(v, ptr, name, errp); } PropertyInfo qdev_prop_uint8 = { .name = "uint8", .get = get_uint8, .set = set_uint8, - .min = 0, - .max = 255, }; /* --- 8bit hex value --- */ @@ -153,8 +139,6 @@ PropertyInfo qdev_prop_hex8 = { .print = print_hex8, .get = get_uint8, .set = set_uint8, - .min = 0, - .max = 255, }; /* --- 16bit integer --- */ @@ -174,34 +158,20 @@ static void set_uint16(Object *obj, Visitor *v, void *opaque, { DeviceState *dev = DEVICE(obj); Property *prop = opaque; - uint16_t value, *ptr = qdev_get_prop_ptr(dev, prop); - Error *local_err = NULL; + uint16_t *ptr = qdev_get_prop_ptr(dev, prop); if (dev->state != DEV_STATE_CREATED) { error_set(errp, QERR_PERMISSION_DENIED); return; } - visit_type_uint16(v, &value, name, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } - if (value >= prop->info->min && value <= prop->info->max) { - *ptr = value; - } else { - error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, - dev->id?:"", name, (int64_t)value, prop->info->min, - prop->info->max); - } + visit_type_uint16(v, ptr, name, errp); } PropertyInfo qdev_prop_uint16 = { .name = "uint16", .get = get_uint16, .set = set_uint16, - .min = 0, - .max = 65535, }; /* --- 32bit integer --- */ @@ -211,10 +181,9 @@ static void get_uint32(Object *obj, Visitor *v, void *opaque, { DeviceState *dev = DEVICE(obj); Property *prop = opaque; - uint32_t value, *ptr = qdev_get_prop_ptr(dev, prop); + uint32_t *ptr = qdev_get_prop_ptr(dev, prop); - value = *ptr; - visit_type_uint32(v, &value, name, errp); + visit_type_uint32(v, ptr, name, errp); } static void set_uint32(Object *obj, Visitor *v, void *opaque, @@ -222,26 +191,14 @@ static void set_uint32(Object *obj, Visitor *v, void *opaque, { DeviceState *dev = DEVICE(obj); Property *prop = opaque; - uint32_t value, *ptr = qdev_get_prop_ptr(dev, prop); - Error *local_err = NULL; + uint32_t *ptr = qdev_get_prop_ptr(dev, prop); if (dev->state != DEV_STATE_CREATED) { error_set(errp, QERR_PERMISSION_DENIED); return; } - visit_type_uint32(v, &value, name, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } - if (value >= prop->info->min && value <= prop->info->max) { - *ptr = value; - } else { - error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, - dev->id?:"", name, (int64_t)value, prop->info->min, - prop->info->max); - } + visit_type_uint32(v, ptr, name, errp); } static void get_int32(Object *obj, Visitor *v, void *opaque, @@ -259,42 +216,26 @@ static void set_int32(Object *obj, Visitor *v, void *opaque, { DeviceState *dev = DEVICE(obj); Property *prop = opaque; - int32_t value, *ptr = qdev_get_prop_ptr(dev, prop); - Error *local_err = NULL; + int32_t *ptr = qdev_get_prop_ptr(dev, prop); if (dev->state != DEV_STATE_CREATED) { error_set(errp, QERR_PERMISSION_DENIED); return; } - visit_type_int32(v, &value, name, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } - if (value >= prop->info->min && value <= prop->info->max) { - *ptr = value; - } else { - error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, - dev->id?:"", name, (int64_t)value, prop->info->min, - prop->info->max); - } + visit_type_int32(v, ptr, name, errp); } PropertyInfo qdev_prop_uint32 = { .name = "uint32", .get = get_uint32, .set = set_uint32, - .min = 0, - .max = 0xFFFFFFFFULL, }; PropertyInfo qdev_prop_int32 = { .name = "int32", .get = get_int32, .set = set_int32, - .min = -0x80000000LL, - .max = 0x7FFFFFFFLL, }; /* --- 32bit hex value --- */ @@ -329,8 +270,6 @@ PropertyInfo qdev_prop_hex32 = { .print = print_hex32, .get = get_uint32, .set = set_uint32, - .min = 0, - .max = 0xFFFFFFFFULL, }; /* --- 64bit integer --- */ @@ -853,7 +792,7 @@ static void set_pci_devfn(Object *obj, Visitor *v, void *opaque, { DeviceState *dev = DEVICE(obj); Property *prop = opaque; - int32_t *ptr = qdev_get_prop_ptr(dev, prop); + int32_t value, *ptr = qdev_get_prop_ptr(dev, prop); unsigned int slot, fn, n; Error *local_err = NULL; char *str; @@ -866,7 +805,17 @@ static void set_pci_devfn(Object *obj, Visitor *v, void *opaque, visit_type_str(v, &str, name, &local_err); if (local_err) { error_free(local_err); - return set_int32(obj, v, opaque, name, errp); + local_err = NULL; + visit_type_int32(v, &value, name, &local_err); + if (local_err) { + error_propagate(errp, local_err); + } else if (value < -1 || value > 255) { + error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null", + "pci_devfn"); + } else { + *ptr = value; + } + return; } if (sscanf(str, "%x.%x%n", &slot, &fn, &n) != 2) { @@ -904,8 +853,6 @@ PropertyInfo qdev_prop_pci_devfn = { .print = print_pci_devfn, .get = get_int32, .set = set_pci_devfn, - .min = -1, - .max = 255, }; /* --- blocksize --- */ @@ -917,6 +864,8 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque, Property *prop = opaque; uint16_t value, *ptr = qdev_get_prop_ptr(dev, prop); Error *local_err = NULL; + const int64_t min = 512; + const int64_t max = 32768; if (dev->state != DEV_STATE_CREATED) { error_set(errp, QERR_PERMISSION_DENIED); @@ -928,10 +877,9 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque, error_propagate(errp, local_err); return; } - if (value < prop->info->min || value > prop->info->max) { + if (value < min || value > max) { error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, - dev->id?:"", name, (int64_t)value, prop->info->min, - prop->info->max); + dev->id?:"", name, (int64_t)value, min, max); return; } @@ -949,8 +897,6 @@ PropertyInfo qdev_prop_blocksize = { .name = "blocksize", .get = get_uint16, .set = set_blocksize, - .min = 512, - .max = 65024, }; /* --- public helpers --- */ -- cgit v1.1