On 2/6/2024 10:57 AM, Jakub Kicinski wrote:
On Mon, 5 Feb 2024 19:37:54 -0800 Alan Brady wrote:
The motivation for this series has two primary goals. We want to enable
support of multiple simultaneous messages and make the channel more
robust. The way it works right now, the driver can only send and receive
a single message at a time and if something goes really wrong, it can
lead to data corruption and strange bugs.
Coccinelle points out some potential places to use min()
testing/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c:1956:17-18: WARNING
opportunity for min()
testing/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c:1271:17-18: WARNING
opportunity for min()
testing/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c:1319:17-18: WARNING
opportunity for min()
testing/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c:2640:17-18: WARNING
opportunity for min()
testing/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c:1295:17-18: WARNING
opportunity for min()
testing/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c:2157:17-18: WARNING
opportunity for min()
testing/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c:3582:17-18: WARNING
opportunity for min()
We did run coccinelle check and see the min suggestions. It's triggering
on these statements I added:
return reply_sz < 0 ? reply_sz : 0;
A min here would change it to:
return min(reply_sz, 0);
I didn't really like that because it's misleading as though we're
returning the size of the reply and might accidentally encourage someone
to change it to a max. Here reply_sz will be negative if an error was
returned from message sending. But this function we only want to return
0 or negative. By being explicit in what we want to do, it seems clearer
to me what the intention is but I could be wrong.
We can definitely change it however if that's preferred here.
