[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