Currently the Clang analyzer will complain about usage of an
uninitialized variable in the classifier. This is a false positive, but
not for a reason that could easily be detectable by clang.

The classifier is not safe for multiple writer threads to use
simultaniously so all callers protect these functions from simultanious
writes. However, this is not so clear from the code's static analysis
alone. To help Clang out here, the n_indicies count is saved onto the
stack instead of accessed from the subtables struct repeatedly.

Signed-off-by: Mike Pattrick <m...@redhat.com>
---
 lib/classifier.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/lib/classifier.c b/lib/classifier.c
index 0729bd190..55f23b976 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -527,6 +527,7 @@ classifier_replace(struct classifier *cls, const struct 
cls_rule *rule,
     struct cls_match *head;
     unsigned int mask_offset;
     size_t n_rules = 0;
+    uint8_t n_indices;
     uint32_t basis;
     uint32_t hash;
     unsigned int i;
@@ -543,7 +544,8 @@ classifier_replace(struct classifier *cls, const struct 
cls_rule *rule,
     /* Compute hashes in segments. */
     basis = 0;
     mask_offset = 0;
-    for (i = 0; i < subtable->n_indices; i++) {
+    n_indices = subtable->n_indices;
+    for (i = 0; i < n_indices; i++) {
         ihash[i] = minimatch_hash_range(&rule->match, subtable->index_maps[i],
                                         &mask_offset, &basis);
     }
@@ -575,7 +577,7 @@ classifier_replace(struct classifier *cls, const struct 
cls_rule *rule,
         }
 
         /* Add new node to segment indices. */
-        for (i = 0; i < subtable->n_indices; i++) {
+        for (i = 0; i < n_indices; i++) {
             ccmap_inc(&subtable->indices[i], ihash[i]);
         }
         n_rules = cmap_insert(&subtable->rules, &new->cmap_node, hash);
@@ -713,6 +715,7 @@ classifier_remove(struct classifier *cls, const struct 
cls_rule *cls_rule)
     struct cls_subtable *subtable;
     uint32_t basis = 0, hash, ihash[CLS_MAX_INDICES];
     unsigned int mask_offset;
+    uint8_t n_indices;
     size_t n_rules;
     unsigned int i;
 
@@ -730,7 +733,8 @@ classifier_remove(struct classifier *cls, const struct 
cls_rule *cls_rule)
     ovs_assert(subtable);
 
     mask_offset = 0;
-    for (i = 0; i < subtable->n_indices; i++) {
+    n_indices = subtable->n_indices;
+    for (i = 0; i < n_indices; i++) {
         ihash[i] = minimatch_hash_range(&cls_rule->match,
                                         subtable->index_maps[i],
                                         &mask_offset, &basis);
@@ -783,7 +787,7 @@ classifier_remove(struct classifier *cls, const struct 
cls_rule *cls_rule)
     }
 
     /* Remove rule node from indices. */
-    for (i = 0; i < subtable->n_indices; i++) {
+    for (i = 0; i < n_indices; i++) {
         ccmap_dec(&subtable->indices[i], ihash[i]);
     }
     n_rules = cmap_remove(&subtable->rules, &rule->cmap_node, hash);
-- 
2.43.5

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to