On 03/12/2024 17:09, Sabrina Dubroca wrote:
> 2024-12-03, 15:58:17 +0100, Antonio Quartulli wrote:
>> On 02/12/2024 16:07, Antonio Quartulli wrote:
>> [...]
>>> +#define ovpn_get_hash_slot(_key, _key_len, _tbl) ({	\
>>> +	typeof(_tbl) *__tbl = &(_tbl);			\
>>> +	jhash(_key, _key_len, 0) % HASH_SIZE(*__tbl);	\
>>> +})
>>> +
>>> +#define ovpn_get_hash_head(_tbl, _key, _key_len) ({		\
>>> +	typeof(_tbl) *__tbl = &(_tbl);				\
>>> +	&(*__tbl)[ovpn_get_hash_slot(_key, _key_len, *__tbl)];	\
>>> +})
>>
>> clang a reporting various warnings like this:
>>
>> ../drivers/net/ovpn/peer.c:406:9: warning: variable '__tbl' is uninitialized
>> when used within its own initialization [-Wuninitialized]
>>    406 |         head = ovpn_get_hash_head(ovpn->peers->by_id, &peer_id,
>>        |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>    407 |                                   sizeof(peer_id));
>>        |                                   ~~~~~~~~~~~~~~~~
>> ../drivers/net/ovpn/peer.c:179:48: note: expanded from macro
>> 'ovpn_get_hash_head'
>>    179 |         &(*__tbl)[ovpn_get_hash_slot(_key, _key_len, *__tbl)];  \
>>        |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
>> ../drivers/net/ovpn/peer.c:173:26: note: expanded from macro
>> 'ovpn_get_hash_slot'
>>    173 |         typeof(_tbl) *__tbl = &(_tbl);                  \
>>        |                       ~~~~~     ^~~~
>>
>> Anybody willing to help me understand this issue?
>>
>> I have troubles figuring out how __tbl is being used uninitialized.
>> I wonder if the parameters naming is fooling clang (or me) somehow.
> 
> Not really a solution to this specific issue, but do you actually need
> ovpn_get_hash_slot as a separate macro? AFAICT all users could also be
> converted to ovpn_get_hash_head, then you can merge ovpn_get_hash_slot
> into ovpn_get_hash_head and maybe clang won't get confused?
> 
> No guarantee that this fixes anything (except saving one or two lines
> in a few functions).

This is what it used to be before (and no error was reported), but I had 
to split the macro because I need to isolate the slot computation for 
nulls comparison. So there are some users for ovpn_get_hash_slot()

I will quickly try changing the naming and see if clang gets happier.

Regards,


-- 
Antonio Quartulli
OpenVPN Inc.