diff mbox series

[v2,1/3] win32: remove usage of attribute gcc_struct

Message ID 20241126211736.122285-2-pierrick.bouvier@linaro.org (mailing list archive)
State New
Headers show
Series Enable clang build on Windows | expand

Commit Message

Pierrick Bouvier Nov. 26, 2024, 9:17 p.m. UTC 
This attribute is not recognized by clang.

An investigation has been performed to ensure this attribute has no
effect on layout of structures we use in QEMU [1], so it's safe to
remove now.

In the future, we'll forbid introducing new bitfields in packed struct.

[1] https://lore.kernel.org/qemu-devel/66c346de-7e20-4831-b3eb-1cda83240af9@linaro.org/

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 meson.build                               | 5 -----
 include/qemu/compiler.h                   | 7 +------
 scripts/cocci-macro-file.h                | 6 +-----
 subprojects/libvhost-user/libvhost-user.h | 6 +-----
 4 files changed, 3 insertions(+), 21 deletions(-)

Comments

Thomas Huth Nov. 27, 2024, 6:34 a.m. UTC | #1 
On 26/11/2024 22.17, Pierrick Bouvier wrote:
> This attribute is not recognized by clang.
> 
> An investigation has been performed to ensure this attribute has no
> effect on layout of structures we use in QEMU [1], so it's safe to
> remove now.
> 
> In the future, we'll forbid introducing new bitfields in packed struct.
> 
> [1] https://lore.kernel.org/qemu-devel/66c346de-7e20-4831-b3eb-1cda83240af9@linaro.org/
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   meson.build                               | 5 -----
>   include/qemu/compiler.h                   | 7 +------
>   scripts/cocci-macro-file.h                | 6 +-----
>   subprojects/libvhost-user/libvhost-user.h | 6 +-----
>   4 files changed, 3 insertions(+), 21 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>
Stefano Garzarella Nov. 27, 2024, 9:20 a.m. UTC | #2 
On Tue, Nov 26, 2024 at 01:17:34PM -0800, Pierrick Bouvier wrote:
>This attribute is not recognized by clang.
>
>An investigation has been performed to ensure this attribute has no
>effect on layout of structures we use in QEMU [1], so it's safe to
>remove now.
>
>In the future, we'll forbid introducing new bitfields in packed struct.

Why?
I suggest reporting in this commit description what you wrote in the
following patch in the documentation (or a summary), because at first
glance I could not understand the correlation between removing
gcc_struct and bitfields in packed struct.

>
>[1] https://lore.kernel.org/qemu-devel/66c346de-7e20-4831-b3eb-1cda83240af9@linaro.org/
>
>Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>---
> meson.build                               | 5 -----
> include/qemu/compiler.h                   | 7 +------
> scripts/cocci-macro-file.h                | 6 +-----
> subprojects/libvhost-user/libvhost-user.h | 6 +-----
> 4 files changed, 3 insertions(+), 21 deletions(-)

The patch LGTM.

Acked-by: Stefano Garzarella <sgarzare@redhat.com>

>
>diff --git a/meson.build b/meson.build
>index e0b880e4e13..fa6d24e1233 100644
>--- a/meson.build
>+++ b/meson.build
>@@ -354,11 +354,6 @@ elif host_os == 'sunos'
>   qemu_common_flags += '-D__EXTENSIONS__'
> elif host_os == 'haiku'
>   qemu_common_flags += ['-DB_USE_POSITIVE_POSIX_ERRORS', '-D_BSD_SOURCE', '-fPIC']
>-elif host_os == 'windows'
>-  if not compiler.compiles('struct x { int y; } __attribute__((gcc_struct));',
>-                           args: '-Werror')
>-    error('Your compiler does not support __attribute__((gcc_struct)) - please use GCC instead of Clang')
>-  endif
> endif
>
> # Choose instruction set (currently x86-only)
>diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
>index c06954ccb41..d904408e5ed 100644
>--- a/include/qemu/compiler.h
>+++ b/include/qemu/compiler.h
>@@ -22,12 +22,7 @@
> #define QEMU_EXTERN_C extern
> #endif
>
>-#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
>-# define QEMU_PACKED __attribute__((gcc_struct, packed))
>-#else
>-# define QEMU_PACKED __attribute__((packed))
>-#endif
>-
>+#define QEMU_PACKED __attribute__((packed))
> #define QEMU_ALIGNED(X) __attribute__((aligned(X)))
>
> #ifndef glue
>diff --git a/scripts/cocci-macro-file.h b/scripts/cocci-macro-file.h
>index d247a5086e9..c64831d5408 100644
>--- a/scripts/cocci-macro-file.h
>+++ b/scripts/cocci-macro-file.h
>@@ -23,11 +23,7 @@
> #define G_GNUC_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
> #define G_GNUC_NULL_TERMINATED __attribute__((sentinel))
>
>-#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
>-# define QEMU_PACKED __attribute__((gcc_struct, packed))
>-#else
>-# define QEMU_PACKED __attribute__((packed))
>-#endif
>+#define QEMU_PACKED __attribute__((packed))
>
> #define cat(x,y) x ## y
> #define cat2(x,y) cat(x,y)
>diff --git a/subprojects/libvhost-user/libvhost-user.h b/subprojects/libvhost-user/libvhost-user.h
>index deb40e77b3f..2ffc58c11b1 100644
>--- a/subprojects/libvhost-user/libvhost-user.h
>+++ b/subprojects/libvhost-user/libvhost-user.h
>@@ -186,11 +186,7 @@ typedef struct VhostUserShared {
>     unsigned char uuid[UUID_LEN];
> } VhostUserShared;
>
>-#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
>-# define VU_PACKED __attribute__((gcc_struct, packed))
>-#else
>-# define VU_PACKED __attribute__((packed))
>-#endif
>+#define VU_PACKED __attribute__((packed))
>
> typedef struct VhostUserMsg {
>     int request;
>-- 
>2.39.5
>
Pierrick Bouvier Nov. 28, 2024, 12:57 a.m. UTC | #3 
On 11/27/24 01:20, Stefano Garzarella wrote:
> On Tue, Nov 26, 2024 at 01:17:34PM -0800, Pierrick Bouvier wrote:
>> This attribute is not recognized by clang.
>>
>> An investigation has been performed to ensure this attribute has no
>> effect on layout of structures we use in QEMU [1], so it's safe to
>> remove now.
>>
>> In the future, we'll forbid introducing new bitfields in packed struct.
> 
> Why?
> I suggest reporting in this commit description what you wrote in the
> following patch in the documentation (or a summary), because at first
> glance I could not understand the correlation between removing
> gcc_struct and bitfields in packed struct.
> 

Thanks,
I'll mention the problem came explicitely from the bitfields.

>>
>> [1] https://lore.kernel.org/qemu-devel/66c346de-7e20-4831-b3eb-1cda83240af9@linaro.org/
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>> meson.build                               | 5 -----
>> include/qemu/compiler.h                   | 7 +------
>> scripts/cocci-macro-file.h                | 6 +-----
>> subprojects/libvhost-user/libvhost-user.h | 6 +-----
>> 4 files changed, 3 insertions(+), 21 deletions(-)
> 
> The patch LGTM.
> 
> Acked-by: Stefano Garzarella <sgarzare@redhat.com>
> 
>>
>> diff --git a/meson.build b/meson.build
>> index e0b880e4e13..fa6d24e1233 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -354,11 +354,6 @@ elif host_os == 'sunos'
>>    qemu_common_flags += '-D__EXTENSIONS__'
>> elif host_os == 'haiku'
>>    qemu_common_flags += ['-DB_USE_POSITIVE_POSIX_ERRORS', '-D_BSD_SOURCE', '-fPIC']
>> -elif host_os == 'windows'
>> -  if not compiler.compiles('struct x { int y; } __attribute__((gcc_struct));',
>> -                           args: '-Werror')
>> -    error('Your compiler does not support __attribute__((gcc_struct)) - please use GCC instead of Clang')
>> -  endif
>> endif
>>
>> # Choose instruction set (currently x86-only)
>> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
>> index c06954ccb41..d904408e5ed 100644
>> --- a/include/qemu/compiler.h
>> +++ b/include/qemu/compiler.h
>> @@ -22,12 +22,7 @@
>> #define QEMU_EXTERN_C extern
>> #endif
>>
>> -#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
>> -# define QEMU_PACKED __attribute__((gcc_struct, packed))
>> -#else
>> -# define QEMU_PACKED __attribute__((packed))
>> -#endif
>> -
>> +#define QEMU_PACKED __attribute__((packed))
>> #define QEMU_ALIGNED(X) __attribute__((aligned(X)))
>>
>> #ifndef glue
>> diff --git a/scripts/cocci-macro-file.h b/scripts/cocci-macro-file.h
>> index d247a5086e9..c64831d5408 100644
>> --- a/scripts/cocci-macro-file.h
>> +++ b/scripts/cocci-macro-file.h
>> @@ -23,11 +23,7 @@
>> #define G_GNUC_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
>> #define G_GNUC_NULL_TERMINATED __attribute__((sentinel))
>>
>> -#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
>> -# define QEMU_PACKED __attribute__((gcc_struct, packed))
>> -#else
>> -# define QEMU_PACKED __attribute__((packed))
>> -#endif
>> +#define QEMU_PACKED __attribute__((packed))
>>
>> #define cat(x,y) x ## y
>> #define cat2(x,y) cat(x,y)
>> diff --git a/subprojects/libvhost-user/libvhost-user.h b/subprojects/libvhost-user/libvhost-user.h
>> index deb40e77b3f..2ffc58c11b1 100644
>> --- a/subprojects/libvhost-user/libvhost-user.h
>> +++ b/subprojects/libvhost-user/libvhost-user.h
>> @@ -186,11 +186,7 @@ typedef struct VhostUserShared {
>>      unsigned char uuid[UUID_LEN];
>> } VhostUserShared;
>>
>> -#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
>> -# define VU_PACKED __attribute__((gcc_struct, packed))
>> -#else
>> -# define VU_PACKED __attribute__((packed))
>> -#endif
>> +#define VU_PACKED __attribute__((packed))
>>
>> typedef struct VhostUserMsg {
>>      int request;
>> -- 
>> 2.39.5
>>
>
diff mbox series

Patch

diff --git a/meson.build b/meson.build
index e0b880e4e13..fa6d24e1233 100644
--- a/meson.build
+++ b/meson.build
@@ -354,11 +354,6 @@  elif host_os == 'sunos'
   qemu_common_flags += '-D__EXTENSIONS__'
 elif host_os == 'haiku'
   qemu_common_flags += ['-DB_USE_POSITIVE_POSIX_ERRORS', '-D_BSD_SOURCE', '-fPIC']
-elif host_os == 'windows'
-  if not compiler.compiles('struct x { int y; } __attribute__((gcc_struct));',
-                           args: '-Werror')
-    error('Your compiler does not support __attribute__((gcc_struct)) - please use GCC instead of Clang')
-  endif
 endif
 
 # Choose instruction set (currently x86-only)
diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index c06954ccb41..d904408e5ed 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -22,12 +22,7 @@ 
 #define QEMU_EXTERN_C extern
 #endif
 
-#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
-# define QEMU_PACKED __attribute__((gcc_struct, packed))
-#else
-# define QEMU_PACKED __attribute__((packed))
-#endif
-
+#define QEMU_PACKED __attribute__((packed))
 #define QEMU_ALIGNED(X) __attribute__((aligned(X)))
 
 #ifndef glue
diff --git a/scripts/cocci-macro-file.h b/scripts/cocci-macro-file.h
index d247a5086e9..c64831d5408 100644
--- a/scripts/cocci-macro-file.h
+++ b/scripts/cocci-macro-file.h
@@ -23,11 +23,7 @@ 
 #define G_GNUC_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
 #define G_GNUC_NULL_TERMINATED __attribute__((sentinel))
 
-#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
-# define QEMU_PACKED __attribute__((gcc_struct, packed))
-#else
-# define QEMU_PACKED __attribute__((packed))
-#endif
+#define QEMU_PACKED __attribute__((packed))
 
 #define cat(x,y) x ## y
 #define cat2(x,y) cat(x,y)
diff --git a/subprojects/libvhost-user/libvhost-user.h b/subprojects/libvhost-user/libvhost-user.h
index deb40e77b3f..2ffc58c11b1 100644
--- a/subprojects/libvhost-user/libvhost-user.h
+++ b/subprojects/libvhost-user/libvhost-user.h
@@ -186,11 +186,7 @@  typedef struct VhostUserShared {
     unsigned char uuid[UUID_LEN];
 } VhostUserShared;
 
-#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
-# define VU_PACKED __attribute__((gcc_struct, packed))
-#else
-# define VU_PACKED __attribute__((packed))
-#endif
+#define VU_PACKED __attribute__((packed))
 
 typedef struct VhostUserMsg {
     int request;