Re: [PATCH] treewide: Replace zero-length arrays with flexible-array member

[Gustavo A. R. Silva]

On 2/11/20 13:38, Greg KH wrote:
> On Tue, Feb 11, 2020 at 11:32:04AM -0800, Kees Cook wrote:
>> On Tue, Feb 11, 2020 at 01:20:36PM -0600, Gustavo A. R. Silva wrote:
>>>
>>>
>>> On 2/11/20 12:32, Greg KH wrote:
>>>> On Tue, Feb 11, 2020 at 11:41:26AM -0600, Gustavo A. R. Silva wrote:
>>>>> The current codebase makes use of the zero-length array language
>>>>> extension to the C90 standard, but the preferred mechanism to declare
>>>>> variable-length types such as these ones is a flexible array member[1][2],
>>>>> introduced in C99:
>>>>>
>>>>> struct foo {
>>>>>         int stuff;
>>>>>         struct boo array[];
>>>>> };
>>>>>
>>>>> By making use of the mechanism above, we will get a compiler warning
>>>>> in case the flexible array does not occur last in the structure, which
>>>>> will help us prevent some kind of undefined behavior bugs from being
>>>>> unadvertenly introduced[3] to the codebase from now on.
>>>>>
>>>>> All these instances of code were found with the help of the following
>>>>> Coccinelle script:
>>>>>
>>>>> @@
>>>>> identifier S, member, array;
>>>>> type T1, T2;
>>>>> @@
>>>>>
>>>>> struct S {
>>>>>   ...
>>>>>   T1 member;
>>>>>   T2 array[
>>>>> - 0
>>>>>   ];
>>>>> };
>>>>>
>>>>> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
>>>>> [2] https://github.com/KSPP/linux/issues/21
>>>>> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
>>>>>
>>>>> NOTE: I'll carry this in my -next tree for the v5.6 merge window.
>>>>
>>>> Why not carve this up into per-subsystem patches so that we can apply
>>>> them to our 5.7-rc1 trees and then you submit the "remaining" that don't
>>>> somehow get merged at that timeframe for 5.7-rc2?
>>>>
>>>
>>> Yep, sounds good. I'll do that.
>>
>> FWIW, I'd just like to point out that since this is a mechanical change
>> with no code generation differences (unlike the pre-C90 1-byte array
>> conversions), it's a way better use of everyone's time to just splat
>> this in all at once.
>>
>> That said, it looks like Gustavo is up for it, but I'd like us to
>> generally consider these kinds of mechanical changes as being easier to
>> manage in a single patch. (Though getting Acks tends to be a bit
>> harder...)
> 
> Hey, if this is such a mechanical patch, let's get it to Linus now,
> what's preventing that from being merged now?
> 

Well, the only thing is that this has never been in linux-next.

Thanks
--
Gustavo