[PATCH] arm: replace typeof() with __typeof__()

[PATCH] arm: replace typeof() with __typeof__()

[Elliott Mitchell]

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(-)

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)                                     \
-- 
2.20.1

Re: [PATCH for-4.15] arm: replace typeof() with __typeof__()

[Andrew Cooper]

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?

Re: [PATCH for-4.15] arm: replace typeof() with __typeof__()

[Stefano Stabellini]

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

Re: [PATCH for-4.15] arm: replace typeof() with __typeof__()

[Elliott Mitchell]

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*.


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445

Re: [PATCH for-4.15] arm: replace typeof() with __typeof__()

[Roger Pau Monné]

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.

Re: [PATCH] arm: replace typeof() with __typeof__()

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 697 bytes --]

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

[-- Attachment #1.1.2: OpenPGP_0xB0DE9DD628BF132F.asc --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH for-4.15] arm: replace typeof() with __typeof__()

[Jan Beulich]

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

Re: [PATCH for-4.15] arm: replace typeof() with __typeof__()

[Ian Jackson]

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.

Re: [PATCH for-4.15] arm: replace typeof() with __typeof__()

[Julien Grall]

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,

-- 
Julien Grall

Re: [PATCH for-4.15] arm: replace typeof() with __typeof__()

[Elliott Mitchell]

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.


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445