Re: [PATCH 2/2] util: Consolidate and improve workarounds for clang-tidy issue 58992

On Tue, 19 Sep 2023 11:08:51 +1000 David Gibson wrote:

On Mon, Sep 18, 2023 at 10:16:08AM +0200, Stefano Brivio wrote:

...

On Fri, 15 Sep 2023 16:43:37 +1000 David Gibson wrote:

...

We have several workarounds for a clang-tidy bug where the checker doesn't recognize that a number of system calls write to - and therefore initialise - a socket address. We can't neatly use a suppression, because the bogus warning shows up some time after the actual system call, when we access a field of the socket address which clang-tidy erroneously thinks is uninitialised.

Consolidate these workarounds into one place by using macros to implement wrappers around affected system calls which add a memset() of the sockaddr to silence clang-tidy. This removes the need for the individual memset() workarounds at the callers - and the somewhat longwinded explanatory comments.

We can then use a #define to not include the hack in "real" builds, but only consider it for clang-tidy.

I'm probably missing something, but wouldn't it be more obvious to conditionally define the wrapper itself? That is,

#ifdef CLANG_TIDY_58992 # define recvfrom(s, buf, len, flags, src, addrlen) \ wrap_recvfrom((s), (buf), (len), (flags), (src), (addrlen)) #endif

instead of doing that in sa_init()?

Eh.. maybe? I was going for minimal differences in the preprocessed code between the two cases, to reduce the chances of missing some unrelated real problem due to the fact we're kind of lying to our static checker.

Ah, okay, I see your point -- in both cases we'd call a function (even though one is going to be inlined, the other one not necessarily)... sure, it makes sense. -- Stefano