| Message ID | 202103092114.129LEgZp059925@m5p.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm: replace typeof() with __typeof__() | expand |
On 08/03/2021 13:36, Elliott Mitchell wrote: > typeof() is available in Xen's build environment, which uses Xen's > compiler. As these headers are public, they need strict standards > conformance. Only __typeof__() is officially standardized. > > A compiler in standards conformance mode should report: > > warning: implicit declaration of function 'typeof' is invalid in C99 > [-Wimplicit-function-declaration] > > (this has been observed with FreeBSD's kernel build environment) > > Based-on-patch-by: Julien Grall <julien@xen.org>, Sun Oct 4 20:33:04 2015 +0100 > Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com> s!arm!xen/public! in the subject seeing as two thirds of the modifications are in non-ARM headers. Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> This wants backporting as a build fix, so should be considered for 4.15 at this point. I wonder why our header checks don't pick this up. Do we need to throw a -pedantic around?
On Tue, 9 Mar 2021, Andrew Cooper wrote: > On 08/03/2021 13:36, Elliott Mitchell wrote: > > typeof() is available in Xen's build environment, which uses Xen's > > compiler. As these headers are public, they need strict standards > > conformance. Only __typeof__() is officially standardized. > > > > A compiler in standards conformance mode should report: > > > > warning: implicit declaration of function 'typeof' is invalid in C99 > > [-Wimplicit-function-declaration] > > > > (this has been observed with FreeBSD's kernel build environment) > > > > Based-on-patch-by: Julien Grall <julien@xen.org>, Sun Oct 4 20:33:04 2015 +0100 > > Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com> > > s!arm!xen/public! in the subject seeing as two thirds of the > modifications are in non-ARM headers. > > Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > This wants backporting as a build fix, so should be considered for 4.15 > at this point. +1
On Tue, Mar 09, 2021 at 09:27:34PM +0000, Andrew Cooper wrote: > On 08/03/2021 13:36, Elliott Mitchell wrote: > > typeof() is available in Xen's build environment, which uses Xen's > > compiler. As these headers are public, they need strict standards > > conformance. Only __typeof__() is officially standardized. > > > > A compiler in standards conformance mode should report: > > > > warning: implicit declaration of function 'typeof' is invalid in C99 > > [-Wimplicit-function-declaration] > > > > (this has been observed with FreeBSD's kernel build environment) > > > > Based-on-patch-by: Julien Grall <julien@xen.org>, Sun Oct 4 20:33:04 2015 +0100 > > Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com> > > s!arm!xen/public! in the subject seeing as two thirds of the > modifications are in non-ARM headers. Gah! Crucial little detail missing when rewriting the subject line. Julien Grall's original patch/commit only did ARM, but when I checked I found the other two and I did them too. > Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> > > This wants backporting as a build fix, so should be considered for 4.15 > at this point. > > I wonder why our header checks don't pick this up.?? Do we need to throw > a -pedantic around? This came up since FreeBSD's kernel build uses Clang with -std=iso9899:1999. When I found FreeBSD was simply copying Xen's headers it was clear this needed to be *here*.
On Tue, Mar 09, 2021 at 07:24:32PM -0800, Elliott Mitchell wrote: > On Tue, Mar 09, 2021 at 09:27:34PM +0000, Andrew Cooper wrote: > > On 08/03/2021 13:36, Elliott Mitchell wrote: > > > typeof() is available in Xen's build environment, which uses Xen's > > > compiler. As these headers are public, they need strict standards > > > conformance. Only __typeof__() is officially standardized. > > > > > > A compiler in standards conformance mode should report: > > > > > > warning: implicit declaration of function 'typeof' is invalid in C99 > > > [-Wimplicit-function-declaration] > > > > > > (this has been observed with FreeBSD's kernel build environment) > > > > > > Based-on-patch-by: Julien Grall <julien@xen.org>, Sun Oct 4 20:33:04 2015 +0100 > > > Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com> > > > > s!arm!xen/public! in the subject seeing as two thirds of the > > modifications are in non-ARM headers. > > Gah! Crucial little detail missing when rewriting the subject line. > Julien Grall's original patch/commit only did ARM, but when I checked I > found the other two and I did them too. > > > > Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> > > > > This wants backporting as a build fix, so should be considered for 4.15 > > at this point. > > > > I wonder why our header checks don't pick this up.?? Do we need to throw > > a -pedantic around? > > This came up since FreeBSD's kernel build uses Clang with > -std=iso9899:1999. When I found FreeBSD was simply copying Xen's headers > it was clear this needed to be *here*. FTR this was never spotted on x86 because we don't make use of any of the macros that use typeof in the kernel. Arm OTOH has a typeof in set_xen_guest_handle_raw which is used by kernel code. Thanks for fixing those. Thanks, Roger.
On 08.03.21 14:36, Elliott Mitchell wrote: > typeof() is available in Xen's build environment, which uses Xen's > compiler. As these headers are public, they need strict standards > conformance. Only __typeof__() is officially standardized. > > A compiler in standards conformance mode should report: > > warning: implicit declaration of function 'typeof' is invalid in C99 > [-Wimplicit-function-declaration] > > (this has been observed with FreeBSD's kernel build environment) > > Based-on-patch-by: Julien Grall <julien@xen.org>, Sun Oct 4 20:33:04 2015 +0100 > Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com> Reviewed-by: Juergen Gross <jgross@suse.com> Juergen
On 09.03.2021 22:27, Andrew Cooper wrote: > On 08/03/2021 13:36, Elliott Mitchell wrote: >> typeof() is available in Xen's build environment, which uses Xen's >> compiler. As these headers are public, they need strict standards >> conformance. Only __typeof__() is officially standardized. >> >> A compiler in standards conformance mode should report: >> >> warning: implicit declaration of function 'typeof' is invalid in C99 >> [-Wimplicit-function-declaration] >> >> (this has been observed with FreeBSD's kernel build environment) >> >> Based-on-patch-by: Julien Grall <julien@xen.org>, Sun Oct 4 20:33:04 2015 +0100 >> Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com> > > s!arm!xen/public! in the subject seeing as two thirds of the > modifications are in non-ARM headers. > > Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> > > This wants backporting as a build fix, so should be considered for 4.15 > at this point. > > I wonder why our header checks don't pick this up. Do we need to throw > a -pedantic around? That's a long-standing issue with the checking: For issues to be found in macros, the macros would actually need to be used. Jan
Jan Beulich writes ("Re: [PATCH for-4.15] arm: replace typeof() with __typeof__()"):
> On 09.03.2021 22:27, Andrew Cooper wrote:
> > This wants backporting as a build fix, so should be considered for 4.15
> > at this point.
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
Ian.
Hi, On 10/03/2021 11:31, Ian Jackson wrote: > Jan Beulich writes ("Re: [PATCH for-4.15] arm: replace typeof() with __typeof__()"): >> On 09.03.2021 22:27, Andrew Cooper wrote: >>> This wants backporting as a build fix, so should be considered for 4.15 >>> at this point. > > Release-Acked-by: Ian Jackson <iwj@xenproject.org> Thanks! I have committed the patch now. Cheers,
On Wed, Mar 10, 2021 at 09:54:57AM +0100, Jan Beulich wrote: > On 09.03.2021 22:27, Andrew Cooper wrote: > > > > I wonder why our header checks don't pick this up.?? Do we need to throw > > a -pedantic around? > > That's a long-standing issue with the checking: For issues to be > found in macros, the macros would actually need to be used. This is key since only the hunk for xen/include/public/arch-arm.h was found during a build. The other two hunks were found while preparing to submit this to the Xen Project since I checked for other occurrences of typeof(). Had I not spent the time to look, the other three uses might have generated 2-3 additional patches in the future. Also notable the ARM portion was originally found more than 5 years ago (between 4.2 and 4.6), so this had been lurking for a long time.
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h index c365b1b39e..713fd65317 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 ) diff --git a/xen/include/public/hvm/save.h b/xen/include/public/hvm/save.h index f72e3a9bc4..bea5e9f50f 100644 --- a/xen/include/public/hvm/save.h +++ b/xen/include/public/hvm/save.h @@ -82,12 +82,12 @@ struct hvm_save_descriptor { struct __HVM_SAVE_TYPE_##_x { _type t; char c[_code]; char cpt[1];} #endif -#define HVM_SAVE_TYPE(_x) typeof (((struct __HVM_SAVE_TYPE_##_x *)(0))->t) +#define HVM_SAVE_TYPE(_x) __typeof__ (((struct __HVM_SAVE_TYPE_##_x *)(0))->t) #define HVM_SAVE_LENGTH(_x) (sizeof (HVM_SAVE_TYPE(_x))) #define HVM_SAVE_CODE(_x) (sizeof (((struct __HVM_SAVE_TYPE_##_x *)(0))->c)) #ifdef __XEN__ -# define HVM_SAVE_TYPE_COMPAT(_x) typeof (((struct __HVM_SAVE_TYPE_COMPAT_##_x *)(0))->t) +# define HVM_SAVE_TYPE_COMPAT(_x) __typeof__ (((struct __HVM_SAVE_TYPE_COMPAT_##_x *)(0))->t) # define HVM_SAVE_LENGTH_COMPAT(_x) (sizeof (HVM_SAVE_TYPE_COMPAT(_x))) # define HVM_SAVE_HAS_COMPAT(_x) (sizeof (((struct __HVM_SAVE_TYPE_##_x *)(0))->cpt)-1) diff --git a/xen/include/public/io/ring.h b/xen/include/public/io/ring.h index d68615ae2f..6a94a9fe4b 100644 --- a/xen/include/public/io/ring.h +++ b/xen/include/public/io/ring.h @@ -242,7 +242,7 @@ typedef struct __name##_back_ring __name##_back_ring_t */ #define RING_COPY_REQUEST(_r, _idx, _req) do { \ /* Use volatile to force the copy into _req. */ \ - *(_req) = *(volatile typeof(_req))RING_GET_REQUEST(_r, _idx); \ + *(_req) = *(volatile __typeof__(_req))RING_GET_REQUEST(_r, _idx); \ } while (0) #define RING_GET_RESPONSE(_r, _idx) \
typeof() is available in Xen's build environment, which uses Xen's compiler. As these headers are public, they need strict standards conformance. Only __typeof__() is officially standardized. A compiler in standards conformance mode should report: warning: implicit declaration of function 'typeof' is invalid in C99 [-Wimplicit-function-declaration] (this has been observed with FreeBSD's kernel build environment) Based-on-patch-by: Julien Grall <julien@xen.org>, Sun Oct 4 20:33:04 2015 +0100 Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com> --- xen/include/public/arch-arm.h | 2 +- xen/include/public/hvm/save.h | 4 ++-- xen/include/public/io/ring.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-)