| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAMZ6Rq+BbLX-FCLUQ+TZuCMs-3FK5EnO_QKcAD+KeLcTEYmB3A@mail.gmail.com>
Date: Thu, 24 Apr 2025 16:43:25 +0900
From: Vincent Mailhol <mailhol.vincent@...adoo.fr>
To: Felix Maurer <fmaurer@...hat.com>
Cc: socketcan@...tkopp.net, mkl@...gutronix.de, shuah@...nel.org,
davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
horms@...nel.org, linux-can@...r.kernel.org, netdev@...r.kernel.org,
linux-kselftest@...r.kernel.org, dcaratti@...hat.com, fstornio@...hat.com
Subject: Re: [PATCH 3/4] selftests: can: Use fixtures in test_raw_filter
On Tue. 22 Apr. 2025 at 21:04, Felix Maurer <fmaurer@...hat.com> wrote:
> Use fixtures in test_raw_filter instead of generating the test inputs
> during execution. This should make tests easier to follow and extend.
>
> Signed-off-by: Felix Maurer <fmaurer@...hat.com>
> ---
> .../selftests/net/can/test_raw_filter.c | 311 ++++++++++++------
> 1 file changed, 211 insertions(+), 100 deletions(-)
>
> diff --git a/tools/testing/selftests/net/can/test_raw_filter.c b/tools/testing/selftests/net/can/test_raw_filter.c
> index 7414b9aef823..7fe11e020a1c 100644
> --- a/tools/testing/selftests/net/can/test_raw_filter.c
> +++ b/tools/testing/selftests/net/can/test_raw_filter.c
> @@ -18,43 +18,13 @@
(...)
> +TEST_F(can_filters, test_filter)
> +{
> + fd_set rdfs;
> + struct timeval tv;
> + struct can_filter rfilter;
> + struct can_frame frame;
> + int have_rx;
> + int rx;
> + int rxbits, rxbitval;
> + int ret;
Can you reduce the scope of the variables? More precisely, can you
move those declarations
fd_set rdfs;
struct timeval tv;
to the do {} while(); loop and move those declarations
struct can_frame frame;
int rxbitval;
to the if (FD_ISSET(self->sock, &rdfs)) block?
> - TH_LOG("testcase %2d filters : can_id = 0x%08X can_mask = 0x%08X",
> - testcase, rfilter.can_id, rfilter.can_mask);
> + rfilter.can_id = variant->id;
> + rfilter.can_mask = variant->mask;
> + setsockopt(self->sock, SOL_CAN_RAW, CAN_RAW_FILTER,
> + &rfilter, sizeof(rfilter));
>
> - TH_LOG("testcase %2d sending patterns...", testcase);
> + TH_LOG("filters: can_id = 0x%08X can_mask = 0x%08X",
> + rfilter.can_id, rfilter.can_mask);
>
> - ret = send_can_frames(s, testcase);
> - ASSERT_EQ(0, ret)
> - TH_LOG("failed to send CAN frames");
> + ret = send_can_frames(self->sock, variant->testcase);
> + ASSERT_EQ(0, ret)
> + TH_LOG("failed to send CAN frames");
>
> - have_rx = 1;
> - rx = 0;
> - rxbits = 0;
> + rx = 0;
> + rxbits = 0;
>
> - while (have_rx) {
> + do {
> + have_rx = 0;
> + FD_ZERO(&rdfs);
> + FD_SET(self->sock, &rdfs);
> + tv.tv_sec = 0;
> + tv.tv_usec = 50000; /* 50ms timeout */
>
> - have_rx = 0;
> - FD_ZERO(&rdfs);
> - FD_SET(s, &rdfs);
> - tv.tv_sec = 0;
> - tv.tv_usec = 50000; /* 50ms timeout */
> + ret = select(self->sock + 1, &rdfs, NULL, NULL, &tv);
> + ASSERT_LE(0, ret)
> + TH_LOG("failed select for frame %d (%d)", rx, errno);
>
> - ret = select(s+1, &rdfs, NULL, NULL, &tv);
> + if (FD_ISSET(self->sock, &rdfs)) {
> + have_rx = 1;
> + ret = read(self->sock, &frame, sizeof(struct can_frame));
> ASSERT_LE(0, ret)
> - TH_LOG("failed select for frame %d (%d)", rx, errno);
> -
> - if (FD_ISSET(s, &rdfs)) {
> - have_rx = 1;
> - ret = read(s, &frame, sizeof(struct can_frame));
^^^^^^^^^^^^^^^^
sizeof(frame)
> - ASSERT_LE(0, ret)
> - TH_LOG("failed to read frame %d (%d)", rx, errno);
> -
> - ASSERT_EQ(ID, frame.can_id & CAN_SFF_MASK)
> - TH_LOG("received wrong can_id");
> - ASSERT_EQ(testcase, frame.data[0])
> - TH_LOG("received wrong test case");
> -
> - /* test & calc rxbits */
> - rxbitval = 1 << ((frame.can_id & (CAN_EFF_FLAG|CAN_RTR_FLAG|CAN_ERR_FLAG)) >> 28);
> -
> - /* only receive a rxbitval once */
> - ASSERT_NE(rxbitval, rxbits & rxbitval)
> - TH_LOG("received rxbitval %d twice", rxbitval);
> - rxbits |= rxbitval;
> - rx++;
> -
> - TH_LOG("testcase %2d rx : can_id = 0x%08X rx = %d rxbits = %d",
> - testcase, frame.can_id, rx, rxbits);
> - }
> + TH_LOG("failed to read frame %d (%d)", rx, errno);
> +
> + ASSERT_EQ(ID, frame.can_id & CAN_SFF_MASK)
> + TH_LOG("received wrong can_id");
> + ASSERT_EQ(variant->testcase, frame.data[0])
> + TH_LOG("received wrong test case");
> +
> + /* test & calc rxbits */
> + rxbitval = 1 << ((frame.can_id & (CAN_EFF_FLAG|CAN_RTR_FLAG|CAN_ERR_FLAG)) >> 28);
> +
> + /* only receive a rxbitval once */
> + ASSERT_NE(rxbitval, rxbits & rxbitval)
> + TH_LOG("received rxbitval %d twice", rxbitval);
> + rxbits |= rxbitval;
> + rx++;
> +
> + TH_LOG("rx: can_id = 0x%08X rx = %d rxbits = %d",
> + frame.can_id, rx, rxbits);
> }
> - /* rx timed out -> check the received results */
> - ASSERT_EQ(rx_res[testcase], rx)
> - TH_LOG("wrong number of received frames %d", testcase);
> - ASSERT_EQ(rxbits_res[testcase], rxbits)
> - TH_LOG("wrong rxbits value in testcase %d", testcase);
> -
> - TH_LOG("testcase %2d ok", testcase);
> - TH_LOG("---");
> - }
> + } while (have_rx);
>
> - close(s);
> - return;
> + /* rx timed out -> check the received results */
> + ASSERT_EQ(variant->exp_num_rx, rx)
> + TH_LOG("wrong number of received frames");
> + ASSERT_EQ(variant->exp_rxbits, rxbits)
> + TH_LOG("wrong rxbits value");
> }
Yours sincerely,
Vincent Mailhol
Powered by blists - more mailing lists