search

zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.08k stars 6.19k forks source link

Change 0-length arrays to flexible arrays #63845

Open Thalley opened 9 months ago

Thalley commented 9 months ago

Is your enhancement proposal related to a problem? Please describe. Searching for \w+\[0\];\n\} in Zephyr yields 195 results (as of 8c6a3dbe37abd9b905ac89a375b97a811470dcbd) of arrays defied with [0], which is not a valid C array as per https://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf section 6.7.5.2 Array declarators which state:

In addition to optional type qualifiers and the keyword static, the [ and ] may delimit an expression or *. If they delimit an expression (which specifies the size of an array), the expression shall have an integer type. If the expression is a constant expression, it shall have a value greater than zero

The [0] is in fact a compiler extension, and since it is used to denote an element with a variable length (as per https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html), we should be able to replace all [0] with [].

Alternatively, we could also look into bounded flexible arrays that they Linux kernel is currently working on implementing: https://people.kernel.org/kees/bounded-flexible-arrays-in-c

Describe the solution you'd like Replace all [0] with [] to be compliant with C99 and newer standards, instead of relying on a compiler extension.

Describe alternatives you've considered Bounded flexible arrays may be a better solution overall, and would replace most current non-bounded flexible arrays.

Additional context The majority of these [0] arrays seems to be in the Bluetooth subsystem.

Thalley commented 2 months ago

There is a recent warning added to GCC that can help check if there are any flexible array members in the middle of a struct, which is a big no-go: https://gcc.gnu.org/onlinedocs/gcc-14.1.0/gcc/Warning-Options.html#index-Wflex-array-member-not-at-end

Thalley commented 2 months ago

See also https://embeddedor.com/slides/2024/eo/eo2024.pdf

ceolin commented 1 week ago

For coincidence I was looking into it. I was looking for ways to improve buffer checking to avoid overflows and that is something that can help. There are some really neat compiler features, unfortunately some of them are not available in our SDK. But they rely on proper arrays declaration.

I am working on it.

ceolin commented 1 week ago

Among all usages, this one is the weirdest and it seems very error prone https://github.com/zephyrproject-rtos/zephyr/blob/main/tests/bluetooth/tester/src/btp/btp_mesh.h#L117-L118

ceolin commented 1 week ago

https://github.com/zephyrproject-rtos/zephyr/pull/75192 to start :)

Thalley commented 1 week ago

Among all usages, this one is the weirdest and it seems very error prone https://github.com/zephyrproject-rtos/zephyr/blob/main/tests/bluetooth/tester/src/btp/btp_mesh.h#L117-L118

That is actively disallowed in C. Empty/zero-length arrays are only ever allowed if it is the last value in a struct IIRC (and then that struct shall always be the last field if embedded in other structs).

Luckily it's just for a test application, but should definitely be fixed (ping @sjanc )

sjanc commented 1 week ago

yeah, I'll fix that

sjanc commented 1 week ago

fix https://github.com/zephyrproject-rtos/zephyr/pull/75242