[ovs-dev] [PATCH ovn] logical-fields: fix memory leak while initializing ovnfield_by_name

[Damijan Skvarc]

ovnfield_by_name is a hash table, intended to quicky find ovn_field by
name from a list of supported ovn_fields. This hash table is initialized
in ovn_init_symtab() function based on ovn_fields const array. In case
ovn_init_symtab() function is called multiple times then hash table is
reinitialized multiple times without deallocating previously allocated
memory. This is actually what happens in ovn_controller by initializing
lflow.symtab and ofctrl.symtab tables.

Since ovnfield_by_name is initialized twice with same values I have
introduced a flag indicating ovnfield_by_name is already initialized
or not and based on this flag hash table is prevented to be initialized
multiple times.

Note that currently ovn_fields array is constituted from one single
entry and thus searching a list of one entry by using helper hash table
is somehow useless. Memory leak could be solved by simply removing
ovnfield_by_name table and do a linear search through a list of single
entry. However I want to keep previous functionality in case ovn_fields
array will be extended somewhen in the future.

Signed-off-by: Damijan Skvarc <damjan.skvarc at gmail.com>
---
 lib/logical-fields.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/lib/logical-fields.c b/lib/logical-fields.c
index 4ad5bf4..62b9a71 100644
--- a/lib/logical-fields.c
+++ b/lib/logical-fields.c
@@ -57,6 +57,23 @@ add_ct_bit(const char *name, int index, struct shash *symtab)
     free(expansion);
 }
 
+
+static void
+init_ovnfield_by_name()
+{
+    static bool initialized = 0;
+
+    if (0 == initialized) {
+        shash_init(&ovnfield_by_name);
+        for (int i = 0; i < OVN_FIELD_N_IDS; i++) {
+            const struct ovn_field *of = &ovn_fields[i];
+            ovs_assert(of->id == i); /* Fields must be in the enum order. */
+            shash_add_once(&ovnfield_by_name, of->name, of);
+        }
+        initialized=1;
+    }
+}
+
 void
 ovn_init_symtab(struct shash *symtab)
 {
@@ -218,12 +235,8 @@ ovn_init_symtab(struct shash *symtab)
     expr_symtab_add_field(symtab, "sctp.src", MFF_SCTP_SRC, "sctp", false);
     expr_symtab_add_field(symtab, "sctp.dst", MFF_SCTP_DST, "sctp", false);
 
-    shash_init(&ovnfield_by_name);
-    for (int i = 0; i < OVN_FIELD_N_IDS; i++) {
-        const struct ovn_field *of = &ovn_fields[i];
-        ovs_assert(of->id == i); /* Fields must be in the enum order. */
-        shash_add_once(&ovnfield_by_name, of->name, of);
-    }
+    init_ovnfield_by_name();
+
     expr_symtab_add_ovn_field(symtab, "icmp4.frag_mtu", OVN_ICMP4_FRAG_MTU);
 }
 
-- 
2.7.4