public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Stefano Brivio <sbrivio@redhat.com>
Cc: passt-dev@passt.top
Subject: Re: [PATCH 2/2] util: Consolidate and improve workarounds for clang-tidy issue 58992
Date: Tue, 19 Sep 2023 11:08:51 +1000	[thread overview]
Message-ID: <ZQj0o8RedsbFQ2zu@zatzit> (raw)
In-Reply-To: <20230918101608.00434c15@elisabeth>

[-- Attachment #1: Type: text/plain, Size: 1860 bytes --]

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.

I don't feel that strongly about it though, so whichever you'd prefer
is fine.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2023-09-19  2:09 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-15  6:43 [PATCH 0/2] Some static checker fixes David Gibson
2023-09-15  6:43 ` [PATCH 1/2] packet: Avoid shadowing index(3) David Gibson
2023-09-18  8:16   ` Stefano Brivio
2023-09-19  1:05     ` David Gibson
2023-09-15  6:43 ` [PATCH 2/2] util: Consolidate and improve workarounds for clang-tidy issue 58992 David Gibson
2023-09-18  8:16   ` Stefano Brivio
2023-09-19  1:08     ` David Gibson [this message]
2023-09-19 22:17       ` Stefano Brivio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZQj0o8RedsbFQ2zu@zatzit \
    --to=david@gibson.dropbear.id.au \
    --cc=passt-dev@passt.top \
    --cc=sbrivio@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).