On Wed, 27 Nov 2013 18:55:57 +0100
Andreas Färber <afaer...@suse.de> wrote:
> Am 16.07.2013 00:25, schrieb Igor Mammedov:
> > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> > ---
> > v2:
> > - rebase on top of hyperv_spinlock_attempts in X86CPU
> > ---
> > target-i386/cpu.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 47 insertions(+), 1 deletion(-)
> >
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index 14e9c7e..00c2882 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -1473,6 +1473,49 @@ static void x86_cpu_get_feature_words(Object *obj,
> > Visitor *v, void *opaque,
> > error_propagate(errp, err);
> > }
> >
> > +static void x86_get_hv_spinlocks(Object *obj, Visitor *v, void *opaque,
> > + const char *name, Error **errp)
> > +{
> > + X86CPU *cpu = X86_CPU(obj);
> > + int64_t value = cpu->hyperv_spinlock_attempts;
> > +
> > + visit_type_int(v, &value, name, errp);
> > +}
> > +
> > +static void x86_set_hv_spinlocks(Object *obj, Visitor *v, void *opaque,
> > + const char *name, Error **errp)
> > +{
> > + const int64_t min = 0xFFF;
> > + const int64_t max = UINT_MAX;
> > + X86CPU *cpu = X86_CPU(obj);
> > + int64_t value;
> > +
> > + visit_type_int(v, &value, name, errp);
> > + if (error_is_set(errp)) {
> > + return;
> > + }
>
> errp may be NULL. And if an Error gets raised here but not set to *errp
> for lack of pointer, value might be uninitialized:
> object_property_parse(obj, "not-a-number", "hv-spinlocks", NULL);
> So we cannot rely on error_is_set(errp) but must use a local variable to
> enforce any return. Fixed on qom-cpu-next as follows:
There is ~29 easily found instances of invalid use in current master, we
probably should fix them all.
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 435b3b9..0a5a4f0 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1611,10 +1611,12 @@ static void x86_set_hv_spinlocks(Object *obj,
> Visitor *v, void *opaque,
> const int64_t min = 0xFFF;
> const int64_t max = UINT_MAX;
> X86CPU *cpu = X86_CPU(obj);
> + Error *err = NULL;
> int64_t value;
>
> - visit_type_int(v, &value, name, errp);
> - if (error_is_set(errp)) {
> + visit_type_int(v, &value, name, &err);
> + if (err) {
> + error_propagate(errp, err);
> return;
> }
>
>
> https://github.com/afaerber/qemu-cpu/commits/qom-cpu-next
Reviewed-By: Igor Mammedov <imamm...@redhat.com>
>
> Regards,
> Andreas
>
>
> > +
> > + if (value < min || value > max) {
> > + error_setg(errp, "Property %s.%s doesn't take value %" PRId64
> > + " (minimum: %" PRId64 ", maximum: %" PRId64 ")",
> > + object_get_typename(obj), name ? name : "null",
> > + value, min, max);
> > + return;
> > + }
> > + cpu->hyperv_spinlock_attempts = value;
> > +}
> > +
> > +static PropertyInfo qdev_prop_spinlocks = {
> > + .name = "int",
> > + .get = x86_get_hv_spinlocks,
> > + .set = x86_set_hv_spinlocks,
> > +};
> > +
> > +static Property cpu_x86_properties[] = {
> > + { .name = "hv-spinlocks", .info = &qdev_prop_spinlocks },
> > + DEFINE_PROP_END_OF_LIST(),
> > +};
> > +
> > static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
> > {
> > x86_def_t *def;
> > @@ -1586,6 +1629,7 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu,
> > char *features, Error **errp)
> > } else if (!strcmp(featurestr, "hv-spinlocks")) {
> > char *err;
> > const int min = 0xFFF;
> > + char num[32];
> > numvalue = strtoul(val, &err, 0);
> > if (!*val || *err) {
> > error_setg(errp, "bad numerical value %s", val);
> > @@ -1597,7 +1641,8 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu,
> > char *features, Error **errp)
> > min);
> > numvalue = min;
> > }
> > - cpu->hyperv_spinlock_attempts = numvalue;
> > + snprintf(num, sizeof(num), "%" PRId32, numvalue);
> > + object_property_parse(OBJECT(cpu), num, featurestr, errp);
> > } else {
> > error_setg(errp, "unrecognized feature %s", featurestr);
> > goto out;
> > @@ -2521,6 +2566,7 @@ static void x86_cpu_common_class_init(ObjectClass
> > *oc, void *data)
> > xcc->parent_realize = dc->realize;
> > dc->realize = x86_cpu_realizefn;
> > dc->bus_type = TYPE_ICC_BUS;
> > + dc->props = cpu_x86_properties;
> >
> > xcc->parent_reset = cc->reset;
> > cc->reset = x86_cpu_reset;
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
--
Regards,
Igor
