Re: [PATCH v2] driver core: Explicitly initialize struct member @data.have_async in __device_attach()

[Zijun Hu <zijun_hu@icloud.com>]

On 2024/8/23 23:55, Dmitry Torokhov wrote:
> On Fri, Aug 23, 2024 at 08:00:14PM +0800, Zijun Hu wrote:
>> From: Zijun Hu <quic_zijuhu@quicinc.com>
>>
>> __device_attach() relies on compiler to implicitly initialize struct
>> member @data.have_async to avoid the member is used before initialization
>> but readers may not understand that, solved by explicitly initializing
>> @data.have_async as well as existing @data.want_async.
> 
> I do not believe this is needed. We require kernel developers be
> familiar with the language of choice for the kernel.
> 
> We have a ton of partial or empty structure initializers in the kernel.
> If we count only empty non-static ones I see:
> 
> dtor@dtor-ws:~/kernel/work $ git grep '= \?{ \?};' | grep -v static | wc -l
> 5707
> 
> Rough count of partial initializers is (might be over eager and some
> of them could be full ones, on the other hand it does not count
> initializers that span multiple lines and start with opening brace
> only):
> 
> dtor@dtor-ws:~/kernel/work $ git grep '= \?{ \..* };' | grep -v static | wc -l
> 1150
> 
> Are you planning to go through all of them and add complete
> initializers? And keep adjusting them when structures will get extended?
> For what gain?
> 
actually, many partial initializers is proper, for example, we don't use
these fields that are implicitly initialized, for example, another usage
of the same same in the file drivers/base/dd.c.

no, let me take time to check if there are other usages in kernel tree
similar the one we discuss.

> There was no readers confusion, you wrote a tool for C static analysis
> that did not follow C standard and gave you a false warning. Please fix
> your tool instead.
> 
>>
>> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
>> ---
>> Changes in v2:
>> - Remove both fix and stable tag
>> - Correct both title and commit messages
>> - Link to v1: https://lore.kernel.org/r/20240823-fix_have_async-v1-1-43a354b6614b@quicinc.com
>> ---
>>  drivers/base/dd.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>> index 9b745ba54de1..b0c44b0846aa 100644
>> --- a/drivers/base/dd.c
>> +++ b/drivers/base/dd.c
>> @@ -1021,6 +1021,7 @@ static int __device_attach(struct device *dev, bool allow_async)
>>  			.dev = dev,
>>  			.check_async = allow_async,
>>  			.want_async = false,
>> +			.have_async = false,
>>  		};
>>  
>>  		if (dev->parent)
>>
>> ---
>> base-commit: 87ee9981d1f86ee9b1623a46c7f9e4ac24461fe4
>> change-id: 20240823-fix_have_async-3a135618d91b
>>
>> Best regards,
>> -- 
>> Zijun Hu <quic_zijuhu@quicinc.com>
>>
> 
> Thanks.
>