Re: [Xen-devel] [PATCH 21/21] xen: more substitutions

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 5 Oct 2012, Jan Beulich wrote:
> >>> On 05.10.12 at 12:38, Ian Campbell <ian.campbell@xxxxxxxxxx> wrote:
> > @@ -4202,7 +4205,8 @@ static int handle_iomem_range(unsigned long s, 
> > unsigned long e, void *p)
> >          ent.addr = (uint64_t)ctxt->s << PAGE_SHIFT;
> >          ent.size = (uint64_t)(s - ctxt->s) << PAGE_SHIFT;
> >          ent.type = E820_RESERVED;
> > -        buffer = guest_handle_cast(ctxt->map.buffer, e820entry_t);
> > +        buffer_t = guest_handle_cast(ctxt->map.buffer, e820entry_t);
> > +        buffer = guest_handle_from_param(buffer_t, e820entry_t);
> 
> So why is this needed in the first place? I suppose it's related
> to guest_handle_cast() returning a _HANDLE_PARAM(), but
> what's the reason for that?
 
Because almost everywhere is useful to get an _HANDLE_PARAM from
guest_handle_cast, except two cases, and this is one of them


> Nor would I expect __copy_to_guest_offset() to by unable to
> deal with one? I.e., can't the infrastructure be made capable
> of dealing with both, so pointless conversions can be avoided?

I would rather have one more line of code but being very obvious about
what I am doing: guest_handle_cast returns a XEN_GUEST_HANDLE_PARAM so
we need to cast that to XEN_GUEST_HANDLE. We do that in the traditional
way: calling guest_handle_from_param.


> Or if not, is there really a point in making these changes on the
> x86 side (they're benign there, and hence only reduce
> readability)?

It makes the code more coherent. I like the principle of least
surprise.
Also I would like that changing the type of XEN_GUEST_HANDLE_PARAM on
x86 would actually work.

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