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

[Stefano Brivio <sbrivio@redhat.com>]

On Tue, 19 Sep 2023 11:08:51 +1000
David Gibson <david@gibson.dropbear.id.au> 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 <david@gibson.dropbear.id.au> 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