[ovs-dev] [PATCH 5/6] test-conntrack: Fix dead store reported by clang.
Ben Pfaff
blp at ovn.org
Wed Jul 12 04:16:47 UTC 2017
On Fri, Jun 23, 2017 at 09:49:19AM +0000, Kavanagh, Mark B wrote:
> >From: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-bounces at openvswitch.org] On Behalf Of
> >Bhanuprakash Bodireddy
> >Sent: Monday, June 19, 2017 7:54 PM
> >To: dev at openvswitch.org
> >Subject: [ovs-dev] [PATCH 5/6] test-conntrack: Fix dead store reported by clang.
> >
> >Clang reports that value store to 'batch_size' is never read.
> >
> >Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy at intel.com>
>
> Hi Bhanu,
>
> LGTM - I also compiled this with gcc, clang, and sparse without issue. Checkpatch reports no obvious problems either.
>
> Acked-by: Mark Kavanagh <mark.b.kavanagh at intel.com>
Thanks for noticing the problem.
Even with this change, batch_size is a write-only variable. Nothing
ever uses it.
I think that the right fix is something more like this. I have not
tested it.
Andy, it looks like you made the previous change here, does this make
sense?
Thanks,
Ben
--8<--------------------------cut here-------------------------->8--
From: Ben Pfaff <blp at ovn.org>
Date: Tue, 11 Jul 2017 21:15:21 -0700
Subject: [PATCH] test-conntrack: Restore packet batching to pcap test.
The test accepted but then ignored the batch count argument.
Reported-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy at intel.com>
CC: Andy Zhou <azhou at ovn.org>
Fixes: 72c84bc2db23 ("dp-packet: Enhance packet batch APIs.")
Signed-off-by: Ben Pfaff <blp at ovn.org>
---
tests/test-conntrack.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/tests/test-conntrack.c b/tests/test-conntrack.c
index f79a9fcc61e3..6a77db8df5ae 100644
--- a/tests/test-conntrack.c
+++ b/tests/test-conntrack.c
@@ -211,17 +211,22 @@ test_pcap(struct ovs_cmdl_context *ctx)
conntrack_init(&ct);
total_count = 0;
- while (!err) {
+ for (;;) {
struct dp_packet *packet;
struct dp_packet_batch pkt_batch_;
struct dp_packet_batch *batch = &pkt_batch_;
dp_packet_batch_init(batch);
- err = ovs_pcap_read(pcap, &packet, NULL);
- if (err) {
+ for (int i = 0; i < batch_size; i++) {
+ err = ovs_pcap_read(pcap, &packet, NULL);
+ if (err) {
+ break;
+ }
+ dp_packet_batch_add(batch, packet);
+ }
+ if (!batch->count) {
break;
}
- dp_packet_batch_add(batch, packet);
pcap_batch_execute_conntrack(&ct, batch);
DP_PACKET_BATCH_FOR_EACH (packet, batch) {
--
2.10.2
More information about the dev
mailing list