Re: [PATCH] cppcheck,valgrind: Don't pass NULL to recv() with MSG_TRUNC

[David Gibson <david@gibson.dropbear.id.au>]

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

On Wed, Nov 15, 2023 at 06:32:59AM +0100, Stefano Brivio wrote:
> 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?

Huh.. I hadn't spotted there was an existing suppression.  I don't
know why that's not working any more, I can have a closer look.

> > 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).

Ok, fair points.  I'll revisit this.

-- 
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 --]