Re: [PATCH net-next v12 13/22] ovpn: implement peer lookup logic

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/12/2024 09:28, Antonio Quartulli wrote:
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.

Indeed it's the declaration of __tbl in ovpn_get_hash_slot() that confuses clang.
I'll rename both __tbl and add a comment to remember why we did that.

Regards,


Regards,



--
Antonio Quartulli
OpenVPN Inc.





[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux