Re: [Xen-devel] [PATCH RFC 3/4] xen: implement SCHEDOP_soft_reset

[Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>]

"Jan Beulich" <JBeulich@xxxxxxxx> writes:

>>>> On 22.06.15 at 18:24, <vkuznets@xxxxxxxxxx> wrote:
>> "Jan Beulich" <JBeulich@xxxxxxxx> writes:
>> 
>>>>>> On 22.06.15 at 18:00, <vkuznets@xxxxxxxxxx> wrote:
>>>> "Jan Beulich" <JBeulich@xxxxxxxx> writes:
>>>> 
>>>>>>>> On 03.06.15 at 15:35, <vkuznets@xxxxxxxxxx> wrote:
>>>>>> @@ -1129,8 +1129,9 @@ void unmap_vcpu_info(struct vcpu *v)
>>>>>>      mfn = v->vcpu_info_mfn;
>>>>>>      unmap_domain_page_global((void *)
>>>>>>                               ((unsigned long)v->vcpu_info & PAGE_MASK));
>>>>>> -
>>>>>> -    v->vcpu_info = &dummy_vcpu_info;
>>>>>> +    v->vcpu_info = ((v->vcpu_id < XEN_LEGACY_MAX_VCPUS)
>>>>>> +                    ? (vcpu_info_t *)&shared_info(d, 
>>>>>> vcpu_info[v->vcpu_id])
>>>>>
>>>>> Is this cast really needed?
>>>>>
>>>> 
>>>> Without it my gcc-4.8.3 complains:
>>>> 
>>>> domain.c: In function âunmap_vcpu_infoâ:
>>>> domain.c:1158:21: error: pointer type mismatch in conditional expression 
>>>> [-Werror]
>>>>                      : &dummy_vcpu_info);
>>>>                      ^
>>>> cc1: all warnings being treated as errors
>>>
>>> Which is the kind of warning one normally should _not_ work
>>> around by adding a cast.
>> 
>> In this (and in alloc_vcpu() from where this expression was copied)
>> particular case this is probably OK: in struct shared_info we have
>> 'struct vcpu_info vcpu_info[XEN_LEGACY_MAX_VCPUS]' as its first
>> member. But I may be missing something..
>
> Did you read the comment accompanying the definition of
> __shared_info()?
>
> The cast is presumably safe here, but it doesn't _look_ safe. And
> for future readers (and future changes) it would be better if it did.

Sorry, it seems I don't undestand the suggestion on how to make this
look better.

With CONFIG_COMPAT in both struct shared_info and struct
compat_shared_info vcpu_info array is of type 'struct vcpu_info[]' but
v->vcpu_info is of type 'vcpu_info_t *' which means union of 'struct
vcpu_info' and 'struct compat_vcpu_info' in CONFIG_COMPAT case. Both
these structures have same size of '64' on x86 so it's really up to its
user how to treat this data...

-- 
  Vitaly

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel