Am 25.04.2014 12:44, schrieb Markus Armbruster:
> Using error_is_set(ERRP) to find out whether a function failed is
> either wrong, fragile, or unnecessarily opaque. It's wrong when ERRP
> may be null, because errors go undetected when it is. It's fragile
> when proving ERRP non-null involves a non-local argument. Else, it's
> unnecessarily opaque (see commit 84d18f0).
>
> I guess the error_is_set(errp) in the ObjectProperty set() methods are
> merely fragile right now, because I can't find a call chain that
> passes a null errp argument.
>
> Make the code more robust and more obviously correct: receive the
> error in a local variable, then propagate it through the parameter.
>
> Signed-off-by: Markus Armbruster <arm...@redhat.com>
Tweaking as follows:
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 5c08611..2e21eba 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -746,11 +746,11 @@ static void set_prop_arraylen(Object *obj, Visitor
*v, void *opaque,
* array-length field in the device struct, we have to create the
* array itself and dynamically add the corresponding properties.
*/
- Error *local_err = NULL;
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
uint32_t *alenptr = qdev_get_prop_ptr(dev, prop);
void **arrayptr = (void *)dev + prop->arrayoffset;
+ Error *local_err = NULL;
void *eltptr;
const char *arrayname;
int i;
diff --git a/hw/misc/tmp105.c b/hw/misc/tmp105.c
index e11657d..636ee97 100644
--- a/hw/misc/tmp105.c
+++ b/hw/misc/tmp105.c
@@ -67,8 +67,8 @@ static void tmp105_get_temperature(Object *obj,
Visitor *v, void *opaque,
static void tmp105_set_temperature(Object *obj, Visitor *v, void *opaque,
const char *name, Error **errp)
{
- Error *local_err = NULL;
TMP105State *s = TMP105(obj);
+ Error *local_err = NULL;
int64_t temp;
visit_type_int(v, &temp, name, &local_err);
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 51f02eb..971a921 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -141,8 +141,8 @@ static void balloon_stats_set_poll_interval(Object
*obj, struct Visitor *v,
void *opaque, const char *name,
Error **errp)
{
- Error *local_err = NULL;
VirtIOBalloon *s = opaque;
+ Error *local_err = NULL;
int64_t value;
visit_type_int(v, &value, name, &local_err);
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 73954a7..8f193a9 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1296,11 +1296,11 @@ static void x86_cpuid_version_get_family(Object
*obj, Visitor *v, void *opaque,
static void x86_cpuid_version_set_family(Object *obj, Visitor *v, void
*opaque,
const char *name, Error **errp)
{
- Error *local_err = NULL;
X86CPU *cpu = X86_CPU(obj);
CPUX86State *env = &cpu->env;
const int64_t min = 0;
const int64_t max = 0xff + 0xf;
+ Error *local_err = NULL;
int64_t value;
visit_type_int(v, &value, name, &local_err);
@@ -1337,11 +1337,11 @@ static void x86_cpuid_version_get_model(Object
*obj, Visitor *v, void *opaque,
static void x86_cpuid_version_set_model(Object *obj, Visitor *v, void
*opaque,
const char *name, Error **errp)
{
- Error *local_err = NULL;
X86CPU *cpu = X86_CPU(obj);
CPUX86State *env = &cpu->env;
const int64_t min = 0;
const int64_t max = 0xff;
+ Error *local_err = NULL;
int64_t value;
visit_type_int(v, &value, name, &local_err);
@@ -1375,11 +1375,11 @@ static void
x86_cpuid_version_set_stepping(Object *obj, Visitor *v,
void *opaque, const char *name,
Error **errp)
{
- Error *local_err = NULL;
X86CPU *cpu = X86_CPU(obj);
CPUX86State *env = &cpu->env;
const int64_t min = 0;
const int64_t max = 0xf;
+ Error *local_err = NULL;
int64_t value;
visit_type_int(v, &value, name, &local_err);
@@ -1514,10 +1514,10 @@ static void x86_cpuid_get_tsc_freq(Object *obj,
Visitor *v, void *opaque,
static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
const char *name, Error **errp)
{
- Error *local_err = NULL;
X86CPU *cpu = X86_CPU(obj);
const int64_t min = 0;
const int64_t max = INT64_MAX;
+ Error *local_err = NULL;
int64_t value;
visit_type_int(v, &value, name, &local_err);
Regards,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
