[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH RFC 3/4] xen: implement SCHEDOP_soft_reset
>>> On 23.06.15 at 14:10, <vkuznets@xxxxxxxxxx> wrote: > "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. I didn't suggest anything in particular; all I care about is for such casts to be avoided. > 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... Sure, but that doesn't make the cast any more safe or look any better. But then again I see we already have a similar construct in alloc_vcpu(), i.e. - okay, keep it as is. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |