public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: Stefano Brivio <sbrivio@redhat.com>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: passt-dev@passt.top
Subject: Re: [PATCH] cppcheck,valgrind: Don't pass NULL to recv() with MSG_TRUNC
Date: Wed, 15 Nov 2023 06:32:59 +0100	[thread overview]
Message-ID: <20231115063259.2c7bc5b0@elisabeth> (raw)
In-Reply-To: <20231115044124.1496698-1-david@gibson.dropbear.id.au>

On Wed, 15 Nov 2023 15:41:24 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> Usually, of course, it's invalid to pass a NULL buffer to recv().  However,
> it's acceptable when using MSG_TRUNC, because that suppresses actually
> writing to the buffer.  So, we pass NULL in tcp_sock_consume().
> 
> Unfortunately, checker tools aren't always aware of that special case: we
> already have a suppression for cppcheck to cover this.  valgrind-3.22.0
> (present in Fedora 39) has a similar problem, generating a spurious warning
> here.

I haven't tried valgrind 3.22 yet, but... do you happen to know why
test/valgrind.supp doesn't cover this anymore?

> We could generate another suppression for valgrind, however, it so happens
> that we already have tcp_buf_discard ready to hand.  If we pass this
> instead of NULL it makes both cppcheck and valgrind happy.  We're still
> using the MSG_TRUNC flag, the kernel doesn't actually have to copy data,
> so we should still have the performance benefits of it.

I'm not enthusiastic about this, because using tcp_buf_discard there
might tell an optimising compiler that it's useful to prefetch it.

We would also pass the actual address of tcp_buf_discard to the kernel,
and I'm not sure if this has further subtle implications on possible
optimisations in the kernel implementation (even though as you said no
data is actually copied).

-- 
Stefano


  reply	other threads:[~2023-11-15  5:34 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-15  4:41 [PATCH] cppcheck,valgrind: Don't pass NULL to recv() with MSG_TRUNC David Gibson
2023-11-15  5:32 ` Stefano Brivio [this message]
2023-11-15  8:11   ` David Gibson
2023-11-16  5:25     ` David Gibson

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=20231115063259.2c7bc5b0@elisabeth \
    --to=sbrivio@redhat.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=passt-dev@passt.top \
    /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).