diff mbox series

[v2,2/3] docs/devel/style: add a section about bitfield, and disallow them for packed structures

Message ID 20241126211736.122285-3-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
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 docs/devel/style.rst | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Peter Maydell Nov. 26, 2024, 9:28 p.m. UTC | #1
On Tue, 26 Nov 2024 at 21:18, Pierrick Bouvier
<pierrick.bouvier@linaro.org> wrote:
>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>  docs/devel/style.rst | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/docs/devel/style.rst b/docs/devel/style.rst
> index 2f68b500798..13cb1ef626b 100644
> --- a/docs/devel/style.rst
> +++ b/docs/devel/style.rst
> @@ -416,6 +416,16 @@ definitions instead of typedefs in headers and function prototypes; this
>  avoids problems with duplicated typedefs and reduces the need to include
>  headers from other headers.
>
> +Bitfields
> +---------
> +
> +C bitfields can be a cause of non-portability issues, especially under windows
> +where `MSVC has a different way to layout them than gcc

"to lay them out"

> +<https://gcc.gnu.org/onlinedocs/gcc/x86-Type-Attributes.html>`_.

We should mention also that the layout is different on big and
little endian hosts.

> +For this reason, we disallow usage of bitfields in packed structures.

maybe add "and in any structures which are supposed to exactly
match a specific layout in guest memory" ?

> +For general usage, using bitfields should be proven to add significant benefits
> +regarding memory usage or usability.

Maybe phrase this as

 We also suggest avoiding bitfields even in structures where
 the exact layout does not matter, unless you can show that
 they provide a significant memory usage or usability benefit.

?

> +
>  Reserved namespaces in C and POSIX
>  ----------------------------------
>
> --
> 2.39.5

thanks
-- PMM
Pierrick Bouvier Nov. 27, 2024, 12:15 a.m. UTC | #2
On 11/26/24 13:28, Peter Maydell wrote:
> On Tue, 26 Nov 2024 at 21:18, Pierrick Bouvier
> <pierrick.bouvier@linaro.org> wrote:
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>   docs/devel/style.rst | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/docs/devel/style.rst b/docs/devel/style.rst
>> index 2f68b500798..13cb1ef626b 100644
>> --- a/docs/devel/style.rst
>> +++ b/docs/devel/style.rst
>> @@ -416,6 +416,16 @@ definitions instead of typedefs in headers and function prototypes; this
>>   avoids problems with duplicated typedefs and reduces the need to include
>>   headers from other headers.
>>
>> +Bitfields
>> +---------
>> +
>> +C bitfields can be a cause of non-portability issues, especially under windows
>> +where `MSVC has a different way to layout them than gcc
> 
> "to lay them out"
> 

Thanks for the language fix to a non-native speaker :).

>> +<https://gcc.gnu.org/onlinedocs/gcc/x86-Type-Attributes.html>`_.
> 
> We should mention also that the layout is different on big and
> little endian hosts.
> 
>> +For this reason, we disallow usage of bitfields in packed structures.
> 
> maybe add "and in any structures which are supposed to exactly
> match a specific layout in guest memory" ?
>

I'll add this.

>> +For general usage, using bitfields should be proven to add significant benefits
>> +regarding memory usage or usability.
> 
> Maybe phrase this as
> 
>   We also suggest avoiding bitfields even in structures where
>   the exact layout does not matter, unless you can show that
>   they provide a significant memory usage or usability benefit.
> 
> ?
> 

Ok!

Should we push further and try to convince people bitfields are not 
worth all the problem that come with them (except when really needed)?

Except for saving memory in *very* specific case (a structure allocated 
tens of millions times for example), I hardly see a benefit vs using 
integer types.

>> +
>>   Reserved namespaces in C and POSIX
>>   ----------------------------------
>>
>> --
>> 2.39.5
> 
> thanks
> -- PMM
diff mbox series

Patch

diff --git a/docs/devel/style.rst b/docs/devel/style.rst
index 2f68b500798..13cb1ef626b 100644
--- a/docs/devel/style.rst
+++ b/docs/devel/style.rst
@@ -416,6 +416,16 @@  definitions instead of typedefs in headers and function prototypes; this
 avoids problems with duplicated typedefs and reduces the need to include
 headers from other headers.
 
+Bitfields
+---------
+
+C bitfields can be a cause of non-portability issues, especially under windows
+where `MSVC has a different way to layout them than gcc
+<https://gcc.gnu.org/onlinedocs/gcc/x86-Type-Attributes.html>`_.
+For this reason, we disallow usage of bitfields in packed structures.
+For general usage, using bitfields should be proven to add significant benefits
+regarding memory usage or usability.
+
 Reserved namespaces in C and POSIX
 ----------------------------------