On Thu, Aug 24, 2023 at 4:25 PM Mahmoud Maatuq
<mahmoudmatook.mm@xxxxxxxxx> wrote:
>
> Fix the following coccicheck warning:
> tools/testing/selftests/net/udpgso_bench_tx.c:297:18-19: WARNING opportunity for min()
> tools/testing/selftests/net/udpgso_bench_tx.c:354:27-28: WARNING opportunity for min()
> tools/testing/selftests/net/so_txtime.c:129:24-26: WARNING opportunity for max()
> tools/testing/selftests/net/so_txtime.c:96:30-31: WARNING opportunity for max()
>
> Signed-off-by: Mahmoud Maatuq <mahmoudmatook.mm@xxxxxxxxx>
> Suggested-by: Willem de Bruijn <willemdebruijn.kernel@xxxxxxxxx>
I did not suggest this change.
> ---
> changes in v2:
> cast var cfg_mss to int to avoid static assertion
> after providing a stricter version of min() that does signedness checking.
> ---
> tools/testing/selftests/net/Makefile | 2 ++
> tools/testing/selftests/net/so_txtime.c | 7 ++++---
> tools/testing/selftests/net/udpgso_bench_tx.c | 6 +++---
> 3 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
> index 7f3ab2a93ed6..a06cc25489f9 100644
> --- a/tools/testing/selftests/net/Makefile
> +++ b/tools/testing/selftests/net/Makefile
> @@ -3,6 +3,8 @@
>
> CFLAGS = -Wall -Wl,--no-as-needed -O2 -g
> CFLAGS += -I../../../../usr/include/ $(KHDR_INCLUDES)
> +# Additional include paths needed by kselftest.h
> +CFLAGS += -I../
Why does kselftest.h need this? It does not include anything else?
I'd just add #include "../kselftests.h" to so_txtime.c and remove the
path change from udpgso_bench_tx.c
Fine with this approach. Just don't understand the comment
>
> TEST_PROGS := run_netsocktests run_afpackettests test_bpf.sh netdevice.sh \
> rtnetlink.sh xfrm_policy.sh test_blackhole_dev.sh
> diff --git a/tools/testing/selftests/net/so_txtime.c b/tools/testing/selftests/net/so_txtime.c
> index 2672ac0b6d1f..2936174e7de4 100644
> --- a/tools/testing/selftests/net/so_txtime.c
> +++ b/tools/testing/selftests/net/so_txtime.c
> @@ -33,6 +33,8 @@
> #include <unistd.h>
> #include <poll.h>
>
> +#include "kselftest.h"
> +
> static int cfg_clockid = CLOCK_TAI;
> static uint16_t cfg_port = 8000;
> static int cfg_variance_us = 4000;
> @@ -93,8 +95,7 @@ static void do_send_one(int fdt, struct timed_send *ts)
> msg.msg_controllen = sizeof(control);
>
> tdeliver = glob_tstart + ts->delay_us * 1000;
> - tdeliver_max = tdeliver_max > tdeliver ?
> - tdeliver_max : tdeliver;
> + tdeliver_max = max(tdeliver_max, tdeliver);
>
> cm = CMSG_FIRSTHDR(&msg);
> cm->cmsg_level = SOL_SOCKET;
> @@ -126,7 +127,7 @@ static void do_recv_one(int fdr, struct timed_send *ts)
> error(1, 0, "read: %dB", ret);
>
> tstop = (gettime_ns(cfg_clockid) - glob_tstart) / 1000;
> - texpect = ts->delay_us >= 0 ? ts->delay_us : 0;
> + texpect = max(ts->delay_us, 0);
>
> fprintf(stderr, "payload:%c delay:%lld expected:%lld (us)\n",
> rbuf[0], (long long)tstop, (long long)texpect);
> diff --git a/tools/testing/selftests/net/udpgso_bench_tx.c b/tools/testing/selftests/net/udpgso_bench_tx.c
> index 477392715a9a..6bd32a312901 100644
> --- a/tools/testing/selftests/net/udpgso_bench_tx.c
> +++ b/tools/testing/selftests/net/udpgso_bench_tx.c
> @@ -25,7 +25,7 @@
> #include <sys/types.h>
> #include <unistd.h>
>
> -#include "../kselftest.h"
> +#include "kselftest.h"
>
> #ifndef ETH_MAX_MTU
> #define ETH_MAX_MTU 0xFFFFU
> @@ -294,7 +294,7 @@ static int send_udp(int fd, char *data)
> total_len = cfg_payload_len;
>
> while (total_len) {
> - len = total_len < cfg_mss ? total_len : cfg_mss;
> + len = min(total_len, (int)cfg_mss);
>
> ret = sendto(fd, data, len, cfg_zerocopy ? MSG_ZEROCOPY : 0,
> cfg_connected ? NULL : (void *)&cfg_dst_addr,
> @@ -351,7 +351,7 @@ static int send_udp_sendmmsg(int fd, char *data)
> error(1, 0, "sendmmsg: exceeds max_nr_msg");
>
> iov[i].iov_base = data + off;
> - iov[i].iov_len = cfg_mss < left ? cfg_mss : left;
> + iov[i].iov_len = min(cfg_mss, left);
>
> mmsgs[i].msg_hdr.msg_iov = iov + i;
> mmsgs[i].msg_hdr.msg_iovlen = 1;
> --
> 2.34.1
>
