Re: [Qemu-devel] [PATCH 02/20] target-i386: convert 'hv_spinlocks' to static property

[Paolo Bonzini <pbonzini@redhat.com>]

Il 27/11/2013 18:55, Andreas Färber ha scritto:
> Am 16.07.2013 00:25, schrieb Igor Mammedov:
>> Signed-off-by: Igor Mammedov <imammedo@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:
> 
> 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;
>      }

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>