On Fri, 8 Mar 2024 11:55:32 +1100
David Gibson <david(a)gibson.dropbear.id.au> wrote:
On Wed, Mar 06, 2024 at 08:08:36AM +0100, Stefano
Brivio wrote:
instead of htons_constant(), which is for...
constants.
Fixes: 5bf200ae8a1a ("tcp, udp: Don't include destination address in partially
precomputed csums")
Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
Reviewed-by: David Gibson <david(a)gibson.dropbear.id.au>
It seems to get the job done, at what's probably negligible practical
cost. My perfectionist side has some misgivings:
AIUI, the point of htons_constant() isn't so much that it *requires* a
constant, but that because it's open-coded in plain arithmetic
operations the compiler will be able to constant fold it, if it is
invoked with a constant parameter.
Right, yes, it doesn't require a constant. Still, I'd argue it's meant
for constants. :)
Since htons() is a library
function, it probably can't be elided in that way. The cost of
htons_constant() is that it may be a less optimal implementation when
the calculation really does need to be done at runtime.
I'm still a bit baffled at that Coverity warning: I can't see why it
doesn't preclude any situation where you want to write out
calculations for clarity, even though you know the result will be a
constant (and you expect the compiler to fold it).
...maybe it actually does preclude any situation like that? This is the
only example we have with __bswap_constant16(), and variables mixed
(ORed) with constants.
Other usages of __bswap_constant16() have just a
variable as argument
(no "problem" with that, of course), and we use __bswap_constant_32()
with constants only.
--
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_!