Re: [Xen-devel] [PATCH v3 4/6] xen: introduce XEN_GUEST_HANDLE_PARAM

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.08.12 at 15:55, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
wrote:
> On Fri, 17 Aug 2012, Ian Campbell wrote:
>> On Fri, 2012-08-17 at 14:47 +0100, Stefano Stabellini wrote:
>> > On Fri, 17 Aug 2012, Jan Beulich wrote:
>> > > >>> On 17.08.12 at 10:02, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
>> > > > On Thu, 2012-08-16 at 18:10 +0100, Stefano Stabellini wrote:
>> > > >> On Thu, 16 Aug 2012, Jan Beulich wrote:
>> > > >> > >>> On 16.08.12 at 17:54, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
>> > > >> > > Seeing the patch I btw realized that there's no easy way to
>> > > >> > > avoid having the type as a second argument in the conversion
>> > > >> > > macros. Nevertheless I still don't like the explicitly specified 
>> > > >> > > type
>> > > >> > > there.
>> > > >> > 
>> > > >> > Btw - on the architecture(s) where the two handles are identical
>> > > >> > I would prefer you to make the conversion functions trivial (and
>> > > >> > thus avoid making use of the "type" parameter), thus allowing
>> > > >> > the type checking to occur that you currently circumvent.
>> > > >> 
>> > > >> OK, I can do that.
>> > > > 
>> > > > Will this result in the type parameter potentially becoming stale?
>> > > > 
>> > > > Adding a redundant pointer compare is a good way to get the compiler to
>> > > > catch this. Smth like;
>> > > > 
>> > > >         /* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */
>> > > >         #define guest_handle_from_param(hnd, type) ({
>> > > >             typeof((hnd).p) _x = (hnd).p;
>> > > >             XEN_GUEST_HANDLE(type) _y;
>> > > >             &_y == &_x;
>> > > >             hnd;
>> > > >          })
>> > > 
>> > > Ah yes, that's a good suggestion.
>> > > 
>> > > > I'm not sure which two pointers of members of the various structs need
>> > > > to be compared, maybe it's actually &_y.p and &hnd.p, but you get the
>> > > > idea...
>> > > 
>> > > Right, comparing (hnd).p with _y.p would be the right thing; no
>> > > need for _x, but some other (mechanical) adjustments would be
>> > > necessary.
>> > 
>> > The _x variable is still useful to avoid multiple evaluations of hnd,
>> > even though I know that this is not a public header.
>> > 
>> > What about the following:
>> > 
>> > /* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
>> > #define guest_handle_to_param(hnd, type) ({                \
>> >     typeof((hnd).p) _x = (hnd).p;                          \
>> >     XEN_GUEST_HANDLE_PARAM(type) _y = { _x };              \
>> >     if (&_x != &_y.p) BUG();                               \
>> 
>> &_x and &_y.p will always be different => this will always BUG().
>>
>> You just need "(&_x == &_y.p)" if the types of _x and _y.p are different
>> then the compiler will error out due to the comparison of differently
>> typed pointers.
> 
> I know what you mean, but we cannot do that because the compiler will
> complain with "statement has no effects".
> So we have to do something like:
> 
> if ((&_x == &_y.p) && 0) BUG();

As done in my other mail - simply cast to void.

Jan


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