[PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Julien Grall]

The keyword typeof is not portable:

/usr/src/freebsd/sys/xen/hypervisor.h:93:2: error: implicit declaration
of function 'typeof' is invalid in C99
[-Werror,-Wimplicit-function-declaration]

Signed-off-by: Julien Grall <julien.grall@citrix.com>

---
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>

    This patch is a candidate to backport in Xen 4.6, 4.5 and 4.4.

    It would be nice to get this small fix before Xen 4.6 is released.
    Without it, it's not possible to build ARM software including Xen
    headers with -std=c99.
---
 xen/include/public/arch-arm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index faad68d..ba3cd42 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -191,7 +191,7 @@
 #define XEN_GUEST_HANDLE_PARAM(name)    __guest_handle_ ## name
 #define set_xen_guest_handle_raw(hnd, val)                  \
     do {                                                    \
-        typeof(&(hnd)) _sxghr_tmp = &(hnd);                 \
+        __typeof__(&(hnd)) _sxghr_tmp = &(hnd);             \
         _sxghr_tmp->q = 0;                                  \
         _sxghr_tmp->p = val;                                \
     } while ( 0 )
-- 
2.1.4

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Jan Beulich]

>>> On 04.10.15 at 21:24, <julien.grall@citrix.com> wrote:
> The keyword typeof is not portable:
> 
> /usr/src/freebsd/sys/xen/hypervisor.h:93:2: error: implicit declaration
> of function 'typeof' is invalid in C99
> [-Werror,-Wimplicit-function-declaration]

Actually, it's worse than that - typeof() is a gcc extension, and we
shouldn't use extensions in public headers without at least having
alternative code for not gcc compatible compilers in place. In fact
we should probably aim at removing the exclusion of public/arch-%
in the ANSI conformance check; IIRC I had to add it because things
wouldn't build without, but with the (then forgotten) goal of dealing
with this properly later on.

Jan

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Julien Grall]

Hi Jan,

On 05/10/15 11:31, Jan Beulich wrote:
>>>> On 04.10.15 at 21:24, <julien.grall@citrix.com> wrote:
>> The keyword typeof is not portable:
>>
>> /usr/src/freebsd/sys/xen/hypervisor.h:93:2: error: implicit declaration
>> of function 'typeof' is invalid in C99
>> [-Werror,-Wimplicit-function-declaration]
> 
> Actually, it's worse than that - typeof() is a gcc extension, and we
> shouldn't use extensions in public headers without at least having
> alternative code for not gcc compatible compilers in place. In fact
> we should probably aim at removing the exclusion of public/arch-%
> in the ANSI conformance check; IIRC I had to add it because things
> wouldn't build without, but with the (then forgotten) goal of dealing
> with this properly later on.

I don't see how header.chk would have catch my issue. The problem is in
the macro set_xen_guest_handle_raw which is not used within the headers
(except by set_xen_guest_handle which is not used at all).

It may be worth to add a dummy .c which call the macros to check they
are ANSI compliant.

Regards,

-- 
Julien Grall

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Jan Beulich]

>>> On 06.10.15 at 19:25, <julien.grall@citrix.com> wrote:
> On 05/10/15 11:31, Jan Beulich wrote:
>>>>> On 04.10.15 at 21:24, <julien.grall@citrix.com> wrote:
>>> The keyword typeof is not portable:
>>>
>>> /usr/src/freebsd/sys/xen/hypervisor.h:93:2: error: implicit declaration
>>> of function 'typeof' is invalid in C99
>>> [-Werror,-Wimplicit-function-declaration]
>> 
>> Actually, it's worse than that - typeof() is a gcc extension, and we
>> shouldn't use extensions in public headers without at least having
>> alternative code for not gcc compatible compilers in place. In fact
>> we should probably aim at removing the exclusion of public/arch-%
>> in the ANSI conformance check; IIRC I had to add it because things
>> wouldn't build without, but with the (then forgotten) goal of dealing
>> with this properly later on.
> 
> I don't see how header.chk would have catch my issue. The problem is in
> the macro set_xen_guest_handle_raw which is not used within the headers
> (except by set_xen_guest_handle which is not used at all).
> 
> It may be worth to add a dummy .c which call the macros to check they
> are ANSI compliant.

Hmm, true, conformance of macros isn't being checked right now
(i.e. we only verify that the header as such compiles, not that
everything in the header can be used). A manually created source
would help only to some degree, as it would need to be kept up to
date with future additions. I.e. the long term solution probably
ought to be an at least partially machine generated source file. I
added this to my todo list (but towards the end of it).

Jan

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Ian Campbell]

On Wed, 2015-10-07 at 00:31 -0600, Jan Beulich wrote:
> > > > On 06.10.15 at 19:25, <julien.grall@citrix.com> wrote:
> > On 05/10/15 11:31, Jan Beulich wrote:
> > > > > > On 04.10.15 at 21:24, <julien.grall@citrix.com> wrote:
> > > > The keyword typeof is not portable:
> > > > 
> > > > /usr/src/freebsd/sys/xen/hypervisor.h:93:2: error: implicit
> > > > declaration
> > > > of function 'typeof' is invalid in C99
> > > > [-Werror,-Wimplicit-function-declaration]
> > > 
> > > Actually, it's worse than that - typeof() is a gcc extension, and we
> > > shouldn't use extensions in public headers without at least having
> > > alternative code for not gcc compatible compilers in place. In fact
> > > we should probably aim at removing the exclusion of public/arch-%
> > > in the ANSI conformance check; IIRC I had to add it because things
> > > wouldn't build without, but with the (then forgotten) goal of dealing
> > > with this properly later on.
> > 
> > I don't see how header.chk would have catch my issue. The problem is in
> > the macro set_xen_guest_handle_raw which is not used within the headers
> > (except by set_xen_guest_handle which is not used at all).
> > 
> > It may be worth to add a dummy .c which call the macros to check they
> > are ANSI compliant.
> 
> Hmm, true, conformance of macros isn't being checked right now
> (i.e. we only verify that the header as such compiles, not that
> everything in the header can be used). A manually created source
> would help only to some degree, as it would need to be kept up to
> date with future additions. I.e. the long term solution probably
> ought to be an at least partially machine generated source file

It ought to be possible to at least automatically check that every
    #define ([A-Za-z_]+)
is matched by a corresponding $1 in the .c file.

> . I added this to my todo list (but towards the end of it).

FWIW I have a similar issue with the headers checks for my split up
libxenctrl stable libraries series, and it is in a similar place on my todo
list...

Ian.

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Wei Liu]

On Sun, Oct 04, 2015 at 08:24:02PM +0100, Julien Grall wrote:
> The keyword typeof is not portable:
> 
> /usr/src/freebsd/sys/xen/hypervisor.h:93:2: error: implicit declaration
> of function 'typeof' is invalid in C99
> [-Werror,-Wimplicit-function-declaration]
> 
> Signed-off-by: Julien Grall <julien.grall@citrix.com>
> 
> ---
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Keir Fraser <keir@xen.org>
> Cc: Tim Deegan <tim@xen.org>
> 
>     This patch is a candidate to backport in Xen 4.6, 4.5 and 4.4.
> 
>     It would be nice to get this small fix before Xen 4.6 is released.
>     Without it, it's not possible to build ARM software including Xen
>     headers with -std=c99.

This patch doesn't fundamentally make the header ISO compliance.
And it's a bit too late for 4.6 now.

Wei.

> ---
>  xen/include/public/arch-arm.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index faad68d..ba3cd42 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -191,7 +191,7 @@
>  #define XEN_GUEST_HANDLE_PARAM(name)    __guest_handle_ ## name
>  #define set_xen_guest_handle_raw(hnd, val)                  \
>      do {                                                    \
> -        typeof(&(hnd)) _sxghr_tmp = &(hnd);                 \
> +        __typeof__(&(hnd)) _sxghr_tmp = &(hnd);             \
>          _sxghr_tmp->q = 0;                                  \
>          _sxghr_tmp->p = val;                                \
>      } while ( 0 )
> -- 
> 2.1.4

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Julien Grall]

On 05/10/2015 14:40, Wei Liu wrote:
> On Sun, Oct 04, 2015 at 08:24:02PM +0100, Julien Grall wrote:
>> The keyword typeof is not portable:
>>
>> /usr/src/freebsd/sys/xen/hypervisor.h:93:2: error: implicit declaration
>> of function 'typeof' is invalid in C99
>> [-Werror,-Wimplicit-function-declaration]
>>
>> Signed-off-by: Julien Grall <julien.grall@citrix.com>
>>
>> ---
>> Cc: Ian Campbell <ian.campbell@citrix.com>
>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>> Cc: Jan Beulich <jbeulich@suse.com>
>> Cc: Keir Fraser <keir@xen.org>
>> Cc: Tim Deegan <tim@xen.org>
>>
>>      This patch is a candidate to backport in Xen 4.6, 4.5 and 4.4.
>>
>>      It would be nice to get this small fix before Xen 4.6 is released.
>>      Without it, it's not possible to build ARM software including Xen
>>      headers with -std=c99.
>
> This patch doesn't fundamentally make the header ISO compliance.

I never said it was fully ISO compliant. It's just enough to build the 
kernel using -std=c99 with Xen headers.

Although I believe this is the only non-ISO code we have within this header.

> And it's a bit too late for 4.6 now.

It will be for Xen 4.6.1 then.

Regards,

-- 
Julien Grall

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Julien Grall]

Hi,

On 04/10/15 20:24, Julien Grall wrote:
> The keyword typeof is not portable:
> 
> /usr/src/freebsd/sys/xen/hypervisor.h:93:2: error: implicit declaration
> of function 'typeof' is invalid in C99
> [-Werror,-Wimplicit-function-declaration]

Ping? Aside the fact that other bits of the header may not be iso
compliant, I still think this patch is valid.

FWIW, it fixed build of the FreeBSD kernel for ARM64 with Xen enabled.

Regards,

-- 
Julien Grall

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Ian Campbell]

On Fri, 2015-10-23 at 14:13 +0100, Julien Grall wrote:
> Hi,
> 
> On 04/10/15 20:24, Julien Grall wrote:
> > The keyword typeof is not portable:
> > 
> > /usr/src/freebsd/sys/xen/hypervisor.h:93:2: error: implicit declaration
> > of function 'typeof' is invalid in C99
> > [-Werror,-Wimplicit-function-declaration]
> 
> Ping? Aside the fact that other bits of the header may not be iso
> compliant, I still think this patch is valid.

Yes, I agree.
Acked-by: Ian Campbell <ian.campbell@citrix.com>

Jan, after your earlier comments are you happy to go ahead with this for
now and sort the other possible issues separately?

> 
> FWIW, it fixed build of the FreeBSD kernel for ARM64 with Xen enabled.
> 
> Regards,
> 

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Jan Beulich]

>>> On 23.10.15 at 15:30, <ian.campbell@citrix.com> wrote:
> On Fri, 2015-10-23 at 14:13 +0100, Julien Grall wrote:
>> Hi,
>> 
>> On 04/10/15 20:24, Julien Grall wrote:
>> > The keyword typeof is not portable:
>> > 
>> > /usr/src/freebsd/sys/xen/hypervisor.h:93:2: error: implicit declaration
>> > of function 'typeof' is invalid in C99
>> > [-Werror,-Wimplicit-function-declaration]
>> 
>> Ping? Aside the fact that other bits of the header may not be iso
>> compliant, I still think this patch is valid.
> 
> Yes, I agree.
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> Jan, after your earlier comments are you happy to go ahead with this for
> now and sort the other possible issues separately?

Well - it's an improvement, sure, so I'm not intending to block it
going in if no better way can be determined in its place right away.
What makes me hesitant is that I'm not sure there indeed will be a
follow up to this any time soon.

Jan

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Julien Grall]

On 23/10/15 14:52, Jan Beulich wrote:
>>>> On 23.10.15 at 15:30, <ian.campbell@citrix.com> wrote:
>> On Fri, 2015-10-23 at 14:13 +0100, Julien Grall wrote:
>>> Hi,
>>>
>>> On 04/10/15 20:24, Julien Grall wrote:
>>>> The keyword typeof is not portable:
>>>>
>>>> /usr/src/freebsd/sys/xen/hypervisor.h:93:2: error: implicit declaration
>>>> of function 'typeof' is invalid in C99
>>>> [-Werror,-Wimplicit-function-declaration]
>>>
>>> Ping? Aside the fact that other bits of the header may not be iso
>>> compliant, I still think this patch is valid.
>>
>> Yes, I agree.
>> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>>
>> Jan, after your earlier comments are you happy to go ahead with this for
>> now and sort the other possible issues separately?
> 
> Well - it's an improvement, sure, so I'm not intending to block it
> going in if no better way can be determined in its place right away.
> What makes me hesitant is that I'm not sure there indeed will be a
> follow up to this any time soon.

TBH, having a script which check the validity of the headers is not in
the top my todo list. Though it would be nice to have it.

But I don't think we should delay this valid patch just because we don't
have time to write a such script.

Regards,

-- 
Julien Grall

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Jan Beulich]

>>> On 23.10.15 at 15:58, <julien.grall@citrix.com> wrote:
> On 23/10/15 14:52, Jan Beulich wrote:
>>>>> On 23.10.15 at 15:30, <ian.campbell@citrix.com> wrote:
>>> On Fri, 2015-10-23 at 14:13 +0100, Julien Grall wrote:
>>>> Hi,
>>>>
>>>> On 04/10/15 20:24, Julien Grall wrote:
>>>>> The keyword typeof is not portable:
>>>>>
>>>>> /usr/src/freebsd/sys/xen/hypervisor.h:93:2: error: implicit declaration
>>>>> of function 'typeof' is invalid in C99
>>>>> [-Werror,-Wimplicit-function-declaration]
>>>>
>>>> Ping? Aside the fact that other bits of the header may not be iso
>>>> compliant, I still think this patch is valid.
>>>
>>> Yes, I agree.
>>> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>>>
>>> Jan, after your earlier comments are you happy to go ahead with this for
>>> now and sort the other possible issues separately?
>> 
>> Well - it's an improvement, sure, so I'm not intending to block it
>> going in if no better way can be determined in its place right away.
>> What makes me hesitant is that I'm not sure there indeed will be a
>> follow up to this any time soon.
> 
> TBH, having a script which check the validity of the headers is not in
> the top my todo list. Though it would be nice to have it.

No, the validating script is a nice-to-have, but nothing more. What
I was referring to was a patch to drop the use of this gcc extension.

> But I don't think we should delay this valid patch just because we don't
> have time to write a such script.

As said - I don't intend to block the patch going in; I'm just not
convinced that allowing it to go in won't result in not follow up at all.

Jan

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Ian Campbell]

On Fri, 2015-10-23 at 08:16 -0600, Jan Beulich wrote:
> > > > On 23.10.15 at 15:58, <julien.grall@citrix.com> wrote:
> > On 23/10/15 14:52, Jan Beulich wrote:
> > > > > > On 23.10.15 at 15:30, <ian.campbell@citrix.com> wrote:
> > > > On Fri, 2015-10-23 at 14:13 +0100, Julien Grall wrote:
> > > > > Hi,
> > > > > 
> > > > > On 04/10/15 20:24, Julien Grall wrote:
> > > > > > The keyword typeof is not portable:
> > > > > > 
> > > > > > /usr/src/freebsd/sys/xen/hypervisor.h:93:2: error: implicit
> > > > > > declaration
> > > > > > of function 'typeof' is invalid in C99
> > > > > > [-Werror,-Wimplicit-function-declaration]
> > > > > 
> > > > > Ping? Aside the fact that other bits of the header may not be iso
> > > > > compliant, I still think this patch is valid.
> > > > 
> > > > Yes, I agree.
> > > > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > > > 
> > > > Jan, after your earlier comments are you happy to go ahead with
> > > > this for
> > > > now and sort the other possible issues separately?
> > > 
> > > Well - it's an improvement, sure, so I'm not intending to block it
> > > going in if no better way can be determined in its place right away.
> > > What makes me hesitant is that I'm not sure there indeed will be a
> > > follow up to this any time soon.
> > 
> > TBH, having a script which check the validity of the headers is not in
> > the top my todo list. Though it would be nice to have it.
> 
> No, the validating script is a nice-to-have, but nothing more. What
> I was referring to was a patch to drop the use of this gcc extension.

Then I'm confused. This patch turns a typeof into a __typeof__. In <
56126D8702000078000A80AC@prv-mh.provo.novell.com> you said "typeof() is a
gcc extension".

Are you now saying that __typeof__ also a gcc extension too?

I was under the impression that __typeof__ was standard (by some cxx at
least) and your mail reinforced that (possibly wrong) impression.

https://gcc.gnu.org/onlinedocs/gcc/Typeof.html also says that "If you are
writing a header file that must work when included in ISO C programs, write
__typeof__ instead of typeof", which also lead me to believe __typeof__ was
OK from this PoV.

Ian.

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Ian Campbell]

On Fri, 2015-10-23 at 15:31 +0100, Ian Campbell wrote:
> On Fri, 2015-10-23 at 08:16 -0600, Jan Beulich wrote:
> > > > > On 23.10.15 at 15:58, <julien.grall@citrix.com> wrote:
> > > On 23/10/15 14:52, Jan Beulich wrote:
> > > > > > > On 23.10.15 at 15:30, <ian.campbell@citrix.com> wrote:
> > > > > On Fri, 2015-10-23 at 14:13 +0100, Julien Grall wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > On 04/10/15 20:24, Julien Grall wrote:
> > > > > > > The keyword typeof is not portable:
> > > > > > > 
> > > > > > > /usr/src/freebsd/sys/xen/hypervisor.h:93:2: error: implicit
> > > > > > > declaration
> > > > > > > of function 'typeof' is invalid in C99
> > > > > > > [-Werror,-Wimplicit-function-declaration]
> > > > > > 
> > > > > > Ping? Aside the fact that other bits of the header may not be
> > > > > > iso
> > > > > > compliant, I still think this patch is valid.
> > > > > 
> > > > > Yes, I agree.
> > > > > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > > > > 
> > > > > Jan, after your earlier comments are you happy to go ahead with
> > > > > this for
> > > > > now and sort the other possible issues separately?
> > > > 
> > > > Well - it's an improvement, sure, so I'm not intending to block it
> > > > going in if no better way can be determined in its place right
> > > > away.
> > > > What makes me hesitant is that I'm not sure there indeed will be a
> > > > follow up to this any time soon.
> > > 
> > > TBH, having a script which check the validity of the headers is not
> > > in
> > > the top my todo list. Though it would be nice to have it.
> > 
> > No, the validating script is a nice-to-have, but nothing more. What
> > I was referring to was a patch to drop the use of this gcc extension.
> 
> Then I'm confused. This patch turns a typeof into a __typeof__. In <
> 56126D8702000078000A80AC@prv-mh.provo.novell.com> you said "typeof() is a
> gcc extension".
> 
> Are you now saying that __typeof__ also a gcc extension too?
> 
> I was under the impression that __typeof__ was standard (by some cxx at
> least) and your mail reinforced that (possibly wrong) impression.

Hrm, it seems I was indeed wrong here and __typeof__ is just an alternative
name for the gcc extension keyword which is not subject to -ansi.

Ian.

> 
> https://gcc.gnu.org/onlinedocs/gcc/Typeof.html also says that "If you are
> writing a header file that must work when included in ISO C programs,
> write
> __typeof__ instead of typeof", which also lead me to believe __typeof__
> was
> OK from this PoV.
> 
> Ian.

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Jan Beulich]

>>> On 23.10.15 at 16:31, <ian.campbell@citrix.com> wrote:
> On Fri, 2015-10-23 at 08:16 -0600, Jan Beulich wrote:
>> > > > On 23.10.15 at 15:58, <julien.grall@citrix.com> wrote:
>> > On 23/10/15 14:52, Jan Beulich wrote:
>> > > > > > On 23.10.15 at 15:30, <ian.campbell@citrix.com> wrote:
>> > > > On Fri, 2015-10-23 at 14:13 +0100, Julien Grall wrote:
>> > > > > Hi,
>> > > > > 
>> > > > > On 04/10/15 20:24, Julien Grall wrote:
>> > > > > > The keyword typeof is not portable:
>> > > > > > 
>> > > > > > /usr/src/freebsd/sys/xen/hypervisor.h:93:2: error: implicit
>> > > > > > declaration
>> > > > > > of function 'typeof' is invalid in C99
>> > > > > > [-Werror,-Wimplicit-function-declaration]
>> > > > > 
>> > > > > Ping? Aside the fact that other bits of the header may not be iso
>> > > > > compliant, I still think this patch is valid.
>> > > > 
>> > > > Yes, I agree.
>> > > > Acked-by: Ian Campbell <ian.campbell@citrix.com>
>> > > > 
>> > > > Jan, after your earlier comments are you happy to go ahead with
>> > > > this for
>> > > > now and sort the other possible issues separately?
>> > > 
>> > > Well - it's an improvement, sure, so I'm not intending to block it
>> > > going in if no better way can be determined in its place right away.
>> > > What makes me hesitant is that I'm not sure there indeed will be a
>> > > follow up to this any time soon.
>> > 
>> > TBH, having a script which check the validity of the headers is not in
>> > the top my todo list. Though it would be nice to have it.
>> 
>> No, the validating script is a nice-to-have, but nothing more. What
>> I was referring to was a patch to drop the use of this gcc extension.
> 
> Then I'm confused. This patch turns a typeof into a __typeof__. In <
> 56126D8702000078000A80AC@prv-mh.provo.novell.com> you said "typeof() is a
> gcc extension".
> 
> Are you now saying that __typeof__ also a gcc extension too?
> 
> I was under the impression that __typeof__ was standard (by some cxx at
> least) and your mail reinforced that (possibly wrong) impression.

There's no typeof or __typeof__ in C11 or any earlier standard.
I'm sorry if earlier replies of mine gave a different impression.

> https://gcc.gnu.org/onlinedocs/gcc/Typeof.html also says that "If you are
> writing a header file that must work when included in ISO C programs, write
> __typeof__ instead of typeof", which also lead me to believe __typeof__ was
> OK from this PoV.

That's solely to prevent name space issues - __typeof__ is a
reserved name, while typeof isn't.

Jan

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Julien Grall]

On 23/10/15 15:37, Jan Beulich wrote:
>>>> On 23.10.15 at 16:31, <ian.campbell@citrix.com> wrote:
>> On Fri, 2015-10-23 at 08:16 -0600, Jan Beulich wrote:
>>> No, the validating script is a nice-to-have, but nothing more. What
>>> I was referring to was a patch to drop the use of this gcc extension.
>>
>> Then I'm confused. This patch turns a typeof into a __typeof__. In <
>> 56126D8702000078000A80AC@prv-mh.provo.novell.com> you said "typeof() is a
>> gcc extension".
>>
>> Are you now saying that __typeof__ also a gcc extension too?
>>
>> I was under the impression that __typeof__ was standard (by some cxx at
>> least) and your mail reinforced that (possibly wrong) impression.
> 
> There's no typeof or __typeof__ in C11 or any earlier standard.
> I'm sorry if earlier replies of mine gave a different impression.
> 
>> https://gcc.gnu.org/onlinedocs/gcc/Typeof.html also says that "If you are
>> writing a header file that must work when included in ISO C programs, write
>> __typeof__ instead of typeof", which also lead me to believe __typeof__ was
>> OK from this PoV.
> 
> That's solely to prevent name space issues - __typeof__ is a
> reserved name, while typeof isn't.

Thank you for the explanation. I think we can do the same as x86 does i.e:

#define set_xen_guest_handle_raw(hnd, val)                  \
    do { if ( sizeof(hnd) == 8 ) *(uint64_t *)&(hnd) = 0;   \
         (hnd).p = val;                                     \
    } while ( 0 )

I will send a new version of this patch.

Regards,


-- 
Julien Grall

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Ian Campbell]

On Fri, 2015-10-23 at 15:44 +0100, Julien Grall wrote:
> On 23/10/15 15:37, Jan Beulich wrote:
> > > > > On 23.10.15 at 16:31, <ian.campbell@citrix.com> wrote:
> > > On Fri, 2015-10-23 at 08:16 -0600, Jan Beulich wrote:
> > > > No, the validating script is a nice-to-have, but nothing more. What
> > > > I was referring to was a patch to drop the use of this gcc
> > > > extension.
> > > 
> > > Then I'm confused. This patch turns a typeof into a __typeof__. In <
> > > 56126D8702000078000A80AC@prv-mh.provo.novell.com> you said "typeof()
> > > is a
> > > gcc extension".
> > > 
> > > Are you now saying that __typeof__ also a gcc extension too?
> > > 
> > > I was under the impression that __typeof__ was standard (by some cxx
> > > at
> > > least) and your mail reinforced that (possibly wrong) impression.
> > 
> > There's no typeof or __typeof__ in C11 or any earlier standard.
> > I'm sorry if earlier replies of mine gave a different impression.
> > 
> > > https://gcc.gnu.org/onlinedocs/gcc/Typeof.html also says that "If you
> > > are
> > > writing a header file that must work when included in ISO C programs,
> > > write
> > > __typeof__ instead of typeof", which also lead me to believe
> > > __typeof__ was
> > > OK from this PoV.
> > 
> > That's solely to prevent name space issues - __typeof__ is a
> > reserved name, while typeof isn't.
> 
> Thank you for the explanation. I think we can do the same as x86 does
> i.e:
> 
> #define set_xen_guest_handle_raw(hnd, val)                  \
>     do { if ( sizeof(hnd) == 8 ) *(uint64_t *)&(hnd) = 0;   \
>          (hnd).p = val;                                     \
>     } while ( 0 )

This evaluates hnd twice, which I assumed we wanted to avoid.

But if that is OK for x86 in this situation then there is no harm doing it
on ARM too[0].

But in that case I think we would just do
	(hnd).q = val ; (hnd).p = val
rather than messing with &, casts and *.

I think x86 does it that way because .q doesn't exist in the
 __guest_handle_foo, only the __guest_handle_64_foo, but it exists in both
on ARM.

We also know that sizeof(hnd) == 8 always on both arm subarches. Maybe it
would be worth checking sizeof(hnd.p) == 8 (i.e. whether the real type
completely fills the padding container), I don't know.

Ian.

[0] But maybe do check for arm specific set_guest_handle(foo++, bar) type
constructs which slipped in...

> 
> I will send a new version of this patch.
> 
> Regards,
> 
> 

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Jan Beulich]

>>> On 23.10.15 at 16:55, <ian.campbell@citrix.com> wrote:
> On Fri, 2015-10-23 at 15:44 +0100, Julien Grall wrote:
>> #define set_xen_guest_handle_raw(hnd, val)                  \
>>     do { if ( sizeof(hnd) == 8 ) *(uint64_t *)&(hnd) = 0;   \
>>          (hnd).p = val;                                     \
>>     } while ( 0 )
> 
> This evaluates hnd twice, which I assumed we wanted to avoid.

Hmm, that's true. We've got a couple of __GNUC__ dependencies
in the headers already - perhaps this is a reason to add one more
(and then also mirror the gcc alternative to x86), keeping the use
of __typeof__() in that case, thus ...

> But if that is OK for x86 in this situation then there is no harm doing it
> on ARM too[0].

... eliminating the need for [0] perhaps entirely. (I also note val
isn't properly parenthesized.)

> But in that case I think we would just do
> 	(hnd).q = val ; (hnd).p = val
> rather than messing with &, casts and *.

If you're sure that val always has the upper 32 bits clear...

> I think x86 does it that way because .q doesn't exist in the
>  __guest_handle_foo, only the __guest_handle_64_foo, but it exists in both
> on ARM.

Right.

Jan

> We also know that sizeof(hnd) == 8 always on both arm subarches. Maybe it
> would be worth checking sizeof(hnd.p) == 8 (i.e. whether the real type
> completely fills the padding container), I don't know.
> 
> Ian.
> 
> [0] But maybe do check for arm specific set_guest_handle(foo++, bar) type
> constructs which slipped in...

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Julien Grall]

Hi,

On 23/10/15 15:55, Ian Campbell wrote:
> On Fri, 2015-10-23 at 15:44 +0100, Julien Grall wrote:
>> On 23/10/15 15:37, Jan Beulich wrote:
>>>>>> On 23.10.15 at 16:31, <ian.campbell@citrix.com> wrote:
>>>> On Fri, 2015-10-23 at 08:16 -0600, Jan Beulich wrote:
>>>>> No, the validating script is a nice-to-have, but nothing more. What
>>>>> I was referring to was a patch to drop the use of this gcc
>>>>> extension.
>>>>
>>>> Then I'm confused. This patch turns a typeof into a __typeof__. In <
>>>> 56126D8702000078000A80AC@prv-mh.provo.novell.com> you said "typeof()
>>>> is a
>>>> gcc extension".
>>>>
>>>> Are you now saying that __typeof__ also a gcc extension too?
>>>>
>>>> I was under the impression that __typeof__ was standard (by some cxx
>>>> at
>>>> least) and your mail reinforced that (possibly wrong) impression.
>>>
>>> There's no typeof or __typeof__ in C11 or any earlier standard.
>>> I'm sorry if earlier replies of mine gave a different impression.
>>>
>>>> https://gcc.gnu.org/onlinedocs/gcc/Typeof.html also says that "If you
>>>> are
>>>> writing a header file that must work when included in ISO C programs,
>>>> write
>>>> __typeof__ instead of typeof", which also lead me to believe
>>>> __typeof__ was
>>>> OK from this PoV.
>>>
>>> That's solely to prevent name space issues - __typeof__ is a
>>> reserved name, while typeof isn't.
>>
>> Thank you for the explanation. I think we can do the same as x86 does
>> i.e:
>>
>> #define set_xen_guest_handle_raw(hnd, val)                  \
>>     do { if ( sizeof(hnd) == 8 ) *(uint64_t *)&(hnd) = 0;   \
>>          (hnd).p = val;                                     \
>>     } while ( 0 )
> 
> This evaluates hnd twice, which I assumed we wanted to avoid.
> 
> But if that is OK for x86 in this situation then there is no harm doing it
> on ARM too[0].
> 
> But in that case I think we would just do
> 	(hnd).q = val ; (hnd).p = val
> rather than messing with &, casts and *.

Which is, based on the ISO C spec [1], unspecified. See 6.2.6.1#7:

"When a value is stored in a member of an object of union type, the
bytes of the object representation that do not correspond to that member
but do correspond to other members take unspecified values, but the
value of the union object shall not thereby become a trap
representation."

I've been looking to the solution suggested by Ian Jackson on IRC
friday. I.e smth like:

typedef union {
 uint64_t actual;
 type *for_check }




#define set_xen_guest_handle_raw(hnd, val) 	\
  do { 						\
	hnd.actual = (uint64_t)(val);		\
	sizeof((val) == ((hnd).for_check)
  }

Unfortunately this may not work because we would have to use for_check
for type safety when the pointer is retrieved as we may not have the
type in hand (for instance in get_xen_guest_handle) and accessing
another member than the currently used is not clearly specified (see [2])

However, IIUC, the get_xen_guest_handle is only exposed to the tools and
therefore possible to modify it in order to pass the type. Am I wrong?
FWIW, I haven't seen any usage in the tools directory.

Note that I've also explored the solution to use a structure for ARM32
with contain a padding. I.e smth like:
   struct {
      type *p;
      uint32_t pad;
   }

But it's not possible to set all the fields in one assignation with
(type) { val, 0 } because we don't have the type in hand in
set_xen_guest_handle_raw.

So if we can modify the get_xen_guest_handle to pass a type, Ian's
solution would be the best way forward. Any opinions?

Regards,

[1] http://www.dii.uchile.cl/~daespino/files/Iso_C_1999_definition.pdf
[2] http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_283.htm

-- 
Julien Grall

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Jan Beulich]

>>> On 26.10.15 at 19:08, <julien.grall@citrix.com> wrote:
> However, IIUC, the get_xen_guest_handle is only exposed to the tools and
> therefore possible to modify it in order to pass the type. Am I wrong?
> FWIW, I haven't seen any usage in the tools directory.

Indeed. Rather than worrying about getting it right, let's just drop it.
Patch sent a minute ago.

Jan

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Julien Grall]

On 27/10/15 08:05, Jan Beulich wrote:
>>>> On 26.10.15 at 19:08, <julien.grall@citrix.com> wrote:
>> However, IIUC, the get_xen_guest_handle is only exposed to the tools and
>> therefore possible to modify it in order to pass the type. Am I wrong?
>> FWIW, I haven't seen any usage in the tools directory.
> 
> Indeed. Rather than worrying about getting it right, let's just drop it.
> Patch sent a minute ago.

On the same note, is there any reason we want to expose
XEN_GUEST_HANDLE_PARAM to the guest?

It has been introduced with ARM to represent a guest pointer as an
hypercall pointer and only used within Xen to differentiate 32bit vs
64bit build.

XEN_GUEST_HANDLE will be different when built for 32-bit/64-bit as will
not really have any meaning from the guest POV.

Regards,


-- 
Julien Grall

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Jan Beulich]

>>> On 28.10.15 at 16:44, <julien.grall@citrix.com> wrote:
> On 27/10/15 08:05, Jan Beulich wrote:
>>>>> On 26.10.15 at 19:08, <julien.grall@citrix.com> wrote:
>>> However, IIUC, the get_xen_guest_handle is only exposed to the tools and
>>> therefore possible to modify it in order to pass the type. Am I wrong?
>>> FWIW, I haven't seen any usage in the tools directory.
>> 
>> Indeed. Rather than worrying about getting it right, let's just drop it.
>> Patch sent a minute ago.
> 
> On the same note, is there any reason we want to expose
> XEN_GUEST_HANDLE_PARAM to the guest?
> 
> It has been introduced with ARM to represent a guest pointer as an
> hypercall pointer and only used within Xen to differentiate 32bit vs
> 64bit build.

I don't see a need for it to be exposed, but I'd like the ARM folks
who introduced it (Stefano iirc) confirm that.

Jan

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Stefano Stabellini]

On Wed, 28 Oct 2015, Jan Beulich wrote:
> >>> On 28.10.15 at 16:44, <julien.grall@citrix.com> wrote:
> > On 27/10/15 08:05, Jan Beulich wrote:
> >>>>> On 26.10.15 at 19:08, <julien.grall@citrix.com> wrote:
> >>> However, IIUC, the get_xen_guest_handle is only exposed to the tools and
> >>> therefore possible to modify it in order to pass the type. Am I wrong?
> >>> FWIW, I haven't seen any usage in the tools directory.
> >> 
> >> Indeed. Rather than worrying about getting it right, let's just drop it.
> >> Patch sent a minute ago.
> > 
> > On the same note, is there any reason we want to expose
> > XEN_GUEST_HANDLE_PARAM to the guest?
> > 
> > It has been introduced with ARM to represent a guest pointer as an
> > hypercall pointer and only used within Xen to differentiate 32bit vs
> > 64bit build.
> 
> I don't see a need for it to be exposed, but I'd like the ARM folks
> who introduced it (Stefano iirc) confirm that.
 
Indeed, it should only be used within the hypervisor.

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Jan Beulich]

>>> On 26.10.15 at 19:08, <julien.grall@citrix.com> wrote:
> On 23/10/15 15:55, Ian Campbell wrote:
>> On Fri, 2015-10-23 at 15:44 +0100, Julien Grall wrote:
>>> Thank you for the explanation. I think we can do the same as x86 does
>>> i.e:
>>>
>>> #define set_xen_guest_handle_raw(hnd, val)                  \
>>>     do { if ( sizeof(hnd) == 8 ) *(uint64_t *)&(hnd) = 0;   \
>>>          (hnd).p = val;                                     \
>>>     } while ( 0 )
>> 
>> This evaluates hnd twice, which I assumed we wanted to avoid.
>> 
>> But if that is OK for x86 in this situation then there is no harm doing it
>> on ARM too[0].
>> 
>> But in that case I think we would just do
>> 	(hnd).q = val ; (hnd).p = val
>> rather than messing with &, casts and *.
> 
> Which is, based on the ISO C spec [1], unspecified. See 6.2.6.1#7:
> 
> "When a value is stored in a member of an object of union type, the
> bytes of the object representation that do not correspond to that member
> but do correspond to other members take unspecified values, but the
> value of the union object shall not thereby become a trap
> representation."

Question is - why would you need both assignments? Afaics the
.q one would suffice.

Jan

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Julien Grall]

Hi Jan,

On 27/10/2015 08:07, Jan Beulich wrote:
>>>> On 26.10.15 at 19:08, <julien.grall@citrix.com> wrote:
>> On 23/10/15 15:55, Ian Campbell wrote:
>>> On Fri, 2015-10-23 at 15:44 +0100, Julien Grall wrote:
>>>> Thank you for the explanation. I think we can do the same as x86 does
>>>> i.e:
>>>>
>>>> #define set_xen_guest_handle_raw(hnd, val)                  \
>>>>      do { if ( sizeof(hnd) == 8 ) *(uint64_t *)&(hnd) = 0;   \
>>>>           (hnd).p = val;                                     \
>>>>      } while ( 0 )
>>>
>>> This evaluates hnd twice, which I assumed we wanted to avoid.
>>>
>>> But if that is OK for x86 in this situation then there is no harm doing it
>>> on ARM too[0].
>>>
>>> But in that case I think we would just do
>>> 	(hnd).q = val ; (hnd).p = val
>>> rather than messing with &, casts and *.
>>
>> Which is, based on the ISO C spec [1], unspecified. See 6.2.6.1#7:
>>
>> "When a value is stored in a member of an object of union type, the
>> bytes of the object representation that do not correspond to that member
>> but do correspond to other members take unspecified values, but the
>> value of the union object shall not thereby become a trap
>> representation."
>
> Question is - why would you need both assignments? Afaics the
> .q one would suffice.

Agree .q would suffice only and only if we replace all the usage of .p 
by .q with a cast.

Otherwise, accessing .p just after .q may be implementation-defined (see 
[1]).

Anyway, as you sent a patch to drop get_xen_guest_handle, I will see to 
implement Ian Jackson suggestion. I.e:

typedef union {
  uint64_t actual;
  type *for_check }

#define set_xen_guest_handle_raw(hnd, val) 	\
   do { 						\
	hnd.actual = (uint64_t)(val);		\
	sizeof((val) == ((hnd).for_check)
   }

Regards,

[1]  http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_283.htm

-- 
Julien Grall

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Ian Campbell]

On Fri, 2015-10-23 at 07:52 -0600, Jan Beulich wrote:
> > > > On 23.10.15 at 15:30, <ian.campbell@citrix.com> wrote:
> > On Fri, 2015-10-23 at 14:13 +0100, Julien Grall wrote:
> > > Hi,
> > > 
> > > On 04/10/15 20:24, Julien Grall wrote:
> > > > The keyword typeof is not portable:
> > > > 
> > > > /usr/src/freebsd/sys/xen/hypervisor.h:93:2: error: implicit
> > > > declaration
> > > > of function 'typeof' is invalid in C99
> > > > [-Werror,-Wimplicit-function-declaration]
> > > 
> > > Ping? Aside the fact that other bits of the header may not be iso
> > > compliant, I still think this patch is valid.
> > 
> > Yes, I agree.
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > 
> > Jan, after your earlier comments are you happy to go ahead with this
> > for
> > now and sort the other possible issues separately?
> 
> Well - it's an improvement, sure, so I'm not intending to block it
> going in if no better way can be determined in its place right away.
> What makes me hesitant is that I'm not sure there indeed will be a
> follow up to this any time soon.

Are you saying with "better way" that Julien's fix is incorrect and that
there is potentially a "proper" fix for this specific case? i.e. a followup
to fix the use of __typeof__ in set_xen_guest_handle_raw which this patch
introduces is expected?

I don't think you are, in which case are you suggesting that having fixed
this one issue that Julien should then be on the hook for fixing all
similar/related issues in these header?

I don't think it is right to mandate that Julien put this followup work
onto his TODO list as a condition of accepting this patch, if this is not a
case of Julien's change being incorrect and requiring a "proper" fix, but
that there are other similar things wrong elsewhere.

Of course if he or anyone else wants to do so voluntarily then that's
great.

Ian.

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Jan Beulich]

>>> On 23.10.15 at 16:03, <ian.campbell@citrix.com> wrote:
> On Fri, 2015-10-23 at 07:52 -0600, Jan Beulich wrote:
>> > > > On 23.10.15 at 15:30, <ian.campbell@citrix.com> wrote:
>> > On Fri, 2015-10-23 at 14:13 +0100, Julien Grall wrote:
>> > > Hi,
>> > > 
>> > > On 04/10/15 20:24, Julien Grall wrote:
>> > > > The keyword typeof is not portable:
>> > > > 
>> > > > /usr/src/freebsd/sys/xen/hypervisor.h:93:2: error: implicit
>> > > > declaration
>> > > > of function 'typeof' is invalid in C99
>> > > > [-Werror,-Wimplicit-function-declaration]
>> > > 
>> > > Ping? Aside the fact that other bits of the header may not be iso
>> > > compliant, I still think this patch is valid.
>> > 
>> > Yes, I agree.
>> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
>> > 
>> > Jan, after your earlier comments are you happy to go ahead with this
>> > for
>> > now and sort the other possible issues separately?
>> 
>> Well - it's an improvement, sure, so I'm not intending to block it
>> going in if no better way can be determined in its place right away.
>> What makes me hesitant is that I'm not sure there indeed will be a
>> follow up to this any time soon.
> 
> Are you saying with "better way" that Julien's fix is incorrect and that
> there is potentially a "proper" fix for this specific case? i.e. a followup
> to fix the use of __typeof__ in set_xen_guest_handle_raw which this patch
> introduces is expected?
> 
> I don't think you are, in which case are you suggesting that having fixed
> this one issue that Julien should then be on the hook for fixing all
> similar/related issues in these header?
> 
> I don't think it is right to mandate that Julien put this followup work
> onto his TODO list as a condition of accepting this patch, if this is not a
> case of Julien's change being incorrect and requiring a "proper" fix, but
> that there are other similar things wrong elsewhere.

I'm not meaning to mandate Julien to do all other follow up work. If
anyone was to be mandated, then whoever introduced use of gcc
extensions into public headers. But as we know, that doesn't
normally work (on ARM perhaps everyone who helped with the
bringup may still be around, but outside of ARM that's less likely, so
arguably not a workable model).

The patch is an improvement, yes. So it may go in if right now
no-one can't think of a proper fix for the problem at hand. But
in the end all the patch does is papering over an issue, instead
of eliminating it. And that's the grounds on which I wasn't (and
still am not really) happy to see it go in.

Jan

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Ian Campbell]

On Fri, 2015-10-23 at 08:24 -0600, Jan Beulich wrote:
> > > > On 23.10.15 at 16:03, <ian.campbell@citrix.com> wrote:
> > On Fri, 2015-10-23 at 07:52 -0600, Jan Beulich wrote:
> > > > > > On 23.10.15 at 15:30, <ian.campbell@citrix.com> wrote:
> > > > On Fri, 2015-10-23 at 14:13 +0100, Julien Grall wrote:
> > > > > Hi,
> > > > > 
> > > > > On 04/10/15 20:24, Julien Grall wrote:
> > > > > > The keyword typeof is not portable:
> > > > > > 
> > > > > > /usr/src/freebsd/sys/xen/hypervisor.h:93:2: error: implicit
> > > > > > declaration
> > > > > > of function 'typeof' is invalid in C99
> > > > > > [-Werror,-Wimplicit-function-declaration]
> > > > > 
> > > > > Ping? Aside the fact that other bits of the header may not be iso
> > > > > compliant, I still think this patch is valid.
> > > > 
> > > > Yes, I agree.
> > > > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > > > 
> > > > Jan, after your earlier comments are you happy to go ahead with
> > > > this
> > > > for
> > > > now and sort the other possible issues separately?
> > > 
> > > Well - it's an improvement, sure, so I'm not intending to block it
> > > going in if no better way can be determined in its place right away.
> > > What makes me hesitant is that I'm not sure there indeed will be a
> > > follow up to this any time soon.
> > 
> > Are you saying with "better way" that Julien's fix is incorrect and
> > that
> > there is potentially a "proper" fix for this specific case? i.e. a
> > followup
> > to fix the use of __typeof__ in set_xen_guest_handle_raw which this
> > patch
> > introduces is expected?
> > 
> > I don't think you are, in which case are you suggesting that having
> > fixed
> > this one issue that Julien should then be on the hook for fixing all
> > similar/related issues in these header?
> > 
> > I don't think it is right to mandate that Julien put this followup work
> > onto his TODO list as a condition of accepting this patch, if this is
> > not a
> > case of Julien's change being incorrect and requiring a "proper" fix,
> > but
> > that there are other similar things wrong elsewhere.
> 
> I'm not meaning to mandate Julien to do all other follow up work. If
> anyone was to be mandated, then whoever introduced use of gcc
> extensions into public headers. But as we know, that doesn't
> normally work (on ARM perhaps everyone who helped with the
> bringup may still be around, but outside of ARM that's less likely, so
> arguably not a workable model).
> 
> The patch is an improvement, yes. So it may go in if right now
> no-one can't think of a proper fix for the problem at hand.

If no-one can think of a proper fix now I'm not sure why we would then
expect they would be able to in a follow up, after all if they could they
would have suggested it here and not in a follow up.

Perhaps this needs to be explicitly asked then, So, given:
#define set_xen_guest_handle_raw(hnd, val)                  \
    do {                                                    \
        typeof(&(hnd)) _sxghr_tmp = &(hnd);                 \
        _sxghr_tmp->q = 0;                                  \
        _sxghr_tmp->p = val;                                \
    } while ( 0 )

where typeof is used to avoid the multiple evaluations of hnd which
assigning to hnd.q and hnd.p would involve, is there some way to do the
same in pure ANSI C?

Changing the arguments to set_xen_guest_handle_raw (e.g. to include a type)
would be a prohibitively large change to all of the callers I think, we
should probably just rule that out now.

>  But
> in the end all the patch does is papering over an issue, instead
> of eliminating it. And that's the grounds on which I wasn't (and
> still am not really) happy to see it go in.

If it is to go in then I think under the circumstances the limitations of
the change merely from typeof to __typeof__ ought to be spelled out. i.e.
that the latter is no more standard, just perhaps somewhat more portable.

Ian.

Re: [PATCH for-4.6] xen/public: arm: Use __typeof__ rather than typeof

[Jan Beulich]

>>> On 23.10.15 at 16:48, <ian.campbell@citrix.com> wrote:
> Perhaps this needs to be explicitly asked then, So, given:
> #define set_xen_guest_handle_raw(hnd, val)                  \
>     do {                                                    \
>         typeof(&(hnd)) _sxghr_tmp = &(hnd);                 \
>         _sxghr_tmp->q = 0;                                  \
>         _sxghr_tmp->p = val;                                \
>     } while ( 0 )
> 
> where typeof is used to avoid the multiple evaluations of hnd which
> assigning to hnd.q and hnd.p would involve, is there some way to do the
> same in pure ANSI C?

As per Julien's earlier reply, perhaps the x86 way is usable for arm
after all.

Jan