ovsdb-server spends a lot of time cloning atoms for various reasons,
e.g. to create a diff of two rows or to clone a row to the transaction.
All atoms, except for strings, contains a simple value that could be
copied in efficient way, but duplicating strings every time has a
significant performance impact.

Introducing a new reference-counted structure 'ovsdb_atom_string'
that allows to not copy strings every time, but just increase a
reference counter.

Changes in ovsdb/ovsdb-idlc.in are a bit brute-force-like, but I
didn't manage to write anything better.  And this part is very hard
to understand and debug, so maybe it's better to keep it in this
more or less understandable shape.

This change allows to increase transaction throughput in benchmarks
up to 2x for standalone databases and 3x for clustered databases, i.e.
number of transactions that ovsdb-server can handle per second.
It also noticeably reduces memory consumption of ovsdb-server.

Next step will be to consolidate this structure with json strings,
so we will not need to duplicate strings while converting database
objects to json and back.

Signed-off-by: Ilya Maximets <i.maxim...@ovn.org>
---

There is a small conflict in lib/db-ctl-base.c with the 'set
optimizations' patch-set, but it can be easily resolved if
someone wants to try these patches together.

 lib/db-ctl-base.c      |  11 +--
 lib/ovsdb-cs.c         |   2 +-
 lib/ovsdb-data.c       |  45 ++++++-----
 lib/ovsdb-data.h       |  29 ++++++-
 lib/ovsdb-idl.c        |   3 +-
 ovsdb/ovsdb-idlc.in    | 179 ++++++++++++++++++++++++++++-------------
 ovsdb/ovsdb-server.c   |   6 +-
 ovsdb/ovsdb-util.c     |  16 ++--
 ovsdb/rbac.c           |   8 +-
 python/ovs/db/data.py  |  43 +++++++---
 python/ovs/db/types.py |   7 +-
 tests/test-ovsdb.c     |  14 ++--
 12 files changed, 244 insertions(+), 119 deletions(-)

diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index 77cc76a9f..713487797 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -247,15 +247,15 @@ record_id_equals(const union ovsdb_atom *name, enum 
ovsdb_atomic_type type,
                  const char *record_id)
 {
     if (type == OVSDB_TYPE_STRING) {
-        if (!strcmp(name->string, record_id)) {
+        if (!strcmp(name->s->string, record_id)) {
             return true;
         }
 
         struct uuid uuid;
         size_t len = strlen(record_id);
         if (len >= 4
-            && uuid_from_string(&uuid, name->string)
-            && !strncmp(name->string, record_id, len)) {
+            && uuid_from_string(&uuid, name->s->string)
+            && !strncmp(name->s->string, record_id, len)) {
             return true;
         }
 
@@ -318,11 +318,12 @@ get_row_by_id(struct ctl_context *ctx,
         if (!id->key) {
             name = datum->n == 1 ? &datum->keys[0] : NULL;
         } else {
-            const union ovsdb_atom key_atom
-                = { .string = CONST_CAST(char *, id->key) };
+            union ovsdb_atom key_atom = {
+                .s = ovsdb_atom_string_create(CONST_CAST(char *, id->key)) };
             unsigned int i = ovsdb_datum_find_key(datum, &key_atom,
                                                   OVSDB_TYPE_STRING);
             name = i == UINT_MAX ? NULL : &datum->values[i];
+            ovsdb_atom_destroy(&key_atom, OVSDB_TYPE_STRING);
         }
         if (!name) {
             continue;
diff --git a/lib/ovsdb-cs.c b/lib/ovsdb-cs.c
index 659d49dbf..fcb6fe1b3 100644
--- a/lib/ovsdb-cs.c
+++ b/lib/ovsdb-cs.c
@@ -1833,7 +1833,7 @@ server_column_get_string(const struct server_row *row,
 {
     ovs_assert(server_columns[index].type.key.type == OVSDB_TYPE_STRING);
     const struct ovsdb_datum *d = &row->data[index];
-    return d->n == 1 ? d->keys[0].string : default_value;
+    return d->n == 1 ? d->keys[0].s->string : default_value;
 }
 
 static bool
diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c
index 1f491a98b..0ed8f01b6 100644
--- a/lib/ovsdb-data.c
+++ b/lib/ovsdb-data.c
@@ -74,7 +74,7 @@ ovsdb_atom_init_default(union ovsdb_atom *atom, enum 
ovsdb_atomic_type type)
         break;
 
     case OVSDB_TYPE_STRING:
-        atom->string = xmemdup("", 1);
+        atom->s = ovsdb_atom_string_create_nocopy(xmemdup("", 1));
         break;
 
     case OVSDB_TYPE_UUID:
@@ -136,7 +136,7 @@ ovsdb_atom_is_default(const union ovsdb_atom *atom,
         return atom->boolean == false;
 
     case OVSDB_TYPE_STRING:
-        return atom->string[0] == '\0';
+        return atom->s->string[0] == '\0';
 
     case OVSDB_TYPE_UUID:
         return uuid_is_zero(&atom->uuid);
@@ -172,7 +172,8 @@ ovsdb_atom_clone(union ovsdb_atom *new, const union 
ovsdb_atom *old,
         break;
 
     case OVSDB_TYPE_STRING:
-        new->string = xstrdup(old->string);
+        new->s = old->s;
+        new->s->n_refs++;
         break;
 
     case OVSDB_TYPE_UUID:
@@ -214,7 +215,7 @@ ovsdb_atom_hash(const union ovsdb_atom *atom, enum 
ovsdb_atomic_type type,
         return hash_boolean(atom->boolean, basis);
 
     case OVSDB_TYPE_STRING:
-        return hash_string(atom->string, basis);
+        return hash_string(atom->s->string, basis);
 
     case OVSDB_TYPE_UUID:
         return hash_int(uuid_hash(&atom->uuid), basis);
@@ -246,7 +247,7 @@ ovsdb_atom_compare_3way(const union ovsdb_atom *a,
         return a->boolean - b->boolean;
 
     case OVSDB_TYPE_STRING:
-        return strcmp(a->string, b->string);
+        return a->s == b->s ? 0 : strcmp(a->s->string, b->s->string);
 
     case OVSDB_TYPE_UUID:
         return uuid_compare_3way(&a->uuid, &b->uuid);
@@ -404,7 +405,7 @@ ovsdb_atom_from_json__(union ovsdb_atom *atom,
 
     case OVSDB_TYPE_STRING:
         if (json->type == JSON_STRING) {
-            atom->string = xstrdup(json->string);
+            atom->s = ovsdb_atom_string_create(json->string);
             return NULL;
         }
         break;
@@ -473,7 +474,7 @@ ovsdb_atom_to_json(const union ovsdb_atom *atom, enum 
ovsdb_atomic_type type)
         return json_boolean_create(atom->boolean);
 
     case OVSDB_TYPE_STRING:
-        return json_string_create(atom->string);
+        return json_string_create(atom->s->string);
 
     case OVSDB_TYPE_UUID:
         return wrap_json("uuid", json_string_create_nocopy(
@@ -551,14 +552,18 @@ ovsdb_atom_from_string__(union ovsdb_atom *atom,
             if (s_len < 2 || s[s_len - 1] != '"') {
                 return xasprintf("%s: missing quote at end of "
                                  "quoted string", s);
-            } else if (!json_string_unescape(s + 1, s_len - 2,
-                                             &atom->string)) {
-                char *error = xasprintf("%s: %s", s, atom->string);
-                free(atom->string);
-                return error;
+            } else {
+                char *res;
+                if (json_string_unescape(s + 1, s_len - 2, &res)) {
+                    atom->s = ovsdb_atom_string_create_nocopy(res);
+                } else {
+                    char *error = xasprintf("%s: %s", s, res);
+                    free(res);
+                    return error;
+                }
             }
         } else {
-            atom->string = xstrdup(s);
+            atom->s = ovsdb_atom_string_create(s);
         }
         break;
 
@@ -721,14 +726,14 @@ ovsdb_atom_to_string(const union ovsdb_atom *atom, enum 
ovsdb_atomic_type type,
         break;
 
     case OVSDB_TYPE_STRING:
-        if (string_needs_quotes(atom->string)) {
+        if (string_needs_quotes(atom->s->string)) {
             struct json json;
 
             json.type = JSON_STRING;
-            json.string = atom->string;
+            json.string = atom->s->string;
             json_to_ds(&json, 0, out);
         } else {
-            ds_put_cstr(out, atom->string);
+            ds_put_cstr(out, atom->s->string);
         }
         break;
 
@@ -750,7 +755,7 @@ ovsdb_atom_to_bare(const union ovsdb_atom *atom, enum 
ovsdb_atomic_type type,
                    struct ds *out)
 {
     if (type == OVSDB_TYPE_STRING) {
-        ds_put_cstr(out, atom->string);
+        ds_put_cstr(out, atom->s->string);
     } else {
         ovsdb_atom_to_string(atom, type, out);
     }
@@ -877,7 +882,7 @@ ovsdb_atom_check_constraints(const union ovsdb_atom *atom,
         return NULL;
 
     case OVSDB_TYPE_STRING:
-        return check_string_constraints(atom->string, &base->string);
+        return check_string_constraints(atom->s->string, &base->string);
 
     case OVSDB_TYPE_UUID:
         return NULL;
@@ -1691,8 +1696,8 @@ ovsdb_datum_from_smap(struct ovsdb_datum *datum, const 
struct smap *smap)
     struct smap_node *node;
     size_t i = 0;
     SMAP_FOR_EACH (node, smap) {
-        datum->keys[i].string = xstrdup(node->key);
-        datum->values[i].string = xstrdup(node->value);
+        datum->keys[i].s = ovsdb_atom_string_create(node->key);
+        datum->values[i].s = ovsdb_atom_string_create(node->value);
         i++;
     }
     ovs_assert(i == datum->n);
diff --git a/lib/ovsdb-data.h b/lib/ovsdb-data.h
index aa035ebad..fa7e0d2fc 100644
--- a/lib/ovsdb-data.h
+++ b/lib/ovsdb-data.h
@@ -20,6 +20,7 @@
 #include "compiler.h"
 #include "ovsdb-types.h"
 #include "openvswitch/shash.h"
+#include "util.h"
 
 #ifdef  __cplusplus
 extern "C" {
@@ -31,12 +32,33 @@ struct ds;
 struct ovsdb_symbol_table;
 struct smap;
 
+struct ovsdb_atom_string {
+    char *string;
+    int n_refs;
+};
+
+static inline struct ovsdb_atom_string *
+ovsdb_atom_string_create_nocopy(char *str)
+{
+    struct ovsdb_atom_string *s = xzalloc(sizeof *s);
+
+    s->string = str;
+    s->n_refs = 1;
+    return s;
+}
+
+static inline struct ovsdb_atom_string *
+ovsdb_atom_string_create(const char *str)
+{
+    return ovsdb_atom_string_create_nocopy(xstrdup(str));
+}
+
 /* One value of an atomic type (given by enum ovs_atomic_type). */
 union ovsdb_atom {
     int64_t integer;
     double real;
     bool boolean;
-    char *string;
+    struct ovsdb_atom_string *s;
     struct uuid uuid;
 };
 
@@ -66,8 +88,9 @@ ovsdb_atom_needs_destruction(enum ovsdb_atomic_type type)
 static inline void
 ovsdb_atom_destroy(union ovsdb_atom *atom, enum ovsdb_atomic_type type)
 {
-    if (type == OVSDB_TYPE_STRING) {
-        free(atom->string);
+    if (type == OVSDB_TYPE_STRING && !--atom->s->n_refs) {
+        free(atom->s->string);
+        free(atom->s);
     }
 }
 
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index d3caccec8..40125f234 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -1951,8 +1951,7 @@ ovsdb_idl_index_destroy_row(const struct ovsdb_idl_row 
*row_)
     BITMAP_FOR_EACH_1 (i, class->n_columns, row->written) {
         c = &class->columns[i];
         (c->unparse) (row);
-        free(row->new_datum[i].values);
-        free(row->new_datum[i].keys);
+        ovsdb_datum_destroy(&row->new_datum[i], &c->type);
     }
     free(row->new_datum);
     free(row->written);
diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
index f19e92915..877f903cb 100755
--- a/ovsdb/ovsdb-idlc.in
+++ b/ovsdb/ovsdb-idlc.in
@@ -577,20 +577,20 @@ static void
                 print("    smap_init(&row->%s);" % columnName)
                 print("    for (size_t i = 0; i < datum->n; i++) {")
                 print("        smap_add(&row->%s," % columnName)
-                print("                 datum->keys[i].string,")
-                print("                 datum->values[i].string);")
+                print("                 datum->keys[i].s->string,")
+                print("                 datum->values[i].s->string);")
                 print("    }")
             elif (type.n_min == 1 and type.n_max == 1) or 
type.is_optional_pointer():
                 print("")
                 print("    if (datum->n >= 1) {")
                 if not type.key.ref_table:
-                    print("        %s = datum->keys[0].%s;" % (keyVar, 
type.key.type.to_string()))
+                    print("        %s = datum->keys[0].%s;" % (keyVar, 
type.key.type.to_Cvalue_string()))
                 else:
                     print("        %s = %s%s_cast(ovsdb_idl_get_row_arc(row_, 
&%stable_%s, &datum->keys[0].uuid));" % (keyVar, prefix, 
type.key.ref_table.name.lower(), prefix, type.key.ref_table.name.lower()))
 
                 if valueVar:
                     if not type.value.ref_table:
-                        print("        %s = datum->values[0].%s;" % (valueVar, 
type.value.type.to_string()))
+                        print("        %s = datum->values[0].%s;" % (valueVar, 
type.value.type.to_Cvalue_string()))
                     else:
                         print("        %s = 
%s%s_cast(ovsdb_idl_get_row_arc(row_, &%stable_%s, &datum->values[0].uuid));" % 
(valueVar, prefix, type.value.ref_table.name.lower(), prefix, 
type.value.ref_table.name.lower()))
                 print("    } else {")
@@ -618,7 +618,7 @@ static void
 """ % (prefix, type.key.ref_table.name.lower(), prefix, 
type.key.ref_table.name.lower(), prefix, type.key.ref_table.name.lower()))
                     keySrc = "keyRow"
                 else:
-                    keySrc = "datum->keys[i].%s" % type.key.type.to_string()
+                    keySrc = "datum->keys[i].%s" % 
type.key.type.to_Cvalue_string()
                 if type.value and type.value.ref_table:
                     print("""\
         struct %s%s *valueRow = %s%s_cast(ovsdb_idl_get_row_arc(row_, 
&%stable_%s, &datum->values[i].uuid));
@@ -628,7 +628,7 @@ static void
 """ % (prefix, type.value.ref_table.name.lower(), prefix, 
type.value.ref_table.name.lower(), prefix, type.value.ref_table.name.lower()))
                     valueSrc = "valueRow"
                 elif valueVar:
-                    valueSrc = "datum->values[i].%s" % 
type.value.type.to_string()
+                    valueSrc = "datum->values[i].%s" % 
type.value.type.to_Cvalue_string()
                 print("        if (!row->n_%s) {" % (columnName))
 
                 print("            %s = xmalloc(%s * sizeof *%s);" % (
@@ -936,45 +936,57 @@ void
         'args': ', '.join(['%(type)s%(name)s'
                            % m for m in members])})
             if type.n_min == 1 and type.n_max == 1:
-                print("    union ovsdb_atom key;")
+                print("    union ovsdb_atom *key = xmalloc(sizeof *key);")
                 if type.value:
-                    print("    union ovsdb_atom value;")
+                    print("    union ovsdb_atom *value = xmalloc(sizeof 
*value);")
                 print("")
                 print("    datum.n = 1;")
-                print("    datum.keys = &key;")
-                print("    " + 
type.key.assign_c_value_casting_away_const("key.%s" % 
type.key.type.to_string(), keyVar))
+                print("    datum.keys = key;")
+                if type.key.type == StringType:
+                    print("    key->s = ovsdb_atom_string_create(" + keyVar + 
");")
+                else:
+                    print("    " + 
type.key.assign_c_value_casting_away_const("key->%s" % 
type.key.type.to_string(), keyVar))
                 if type.value:
-                    print("    datum.values = &value;")
-                    print("    "+ 
type.value.assign_c_value_casting_away_const("value.%s" % 
type.value.type.to_string(), valueVar))
+                    print("    datum.values = value;")
+                    if type.value.type == StringType:
+                        print("    value->s = ovsdb_atom_string_create(" + 
valueVar + ");")
+                    else:
+                        print("    "+ 
type.value.assign_c_value_casting_away_const("value->%s" % 
type.value.type.to_string(), valueVar))
                 else:
                     print("    datum.values = NULL;")
-                txn_write_func = "ovsdb_idl_txn_write_clone"
+                txn_write_func = "ovsdb_idl_txn_write"
             elif type.is_optional_pointer():
-                print("    union ovsdb_atom key;")
                 print("")
                 print("    if (%s) {" % keyVar)
+                print("        union ovsdb_atom *key = xmalloc(sizeof *key);")
                 print("        datum.n = 1;")
-                print("        datum.keys = &key;")
-                print("        " + 
type.key.assign_c_value_casting_away_const("key.%s" % 
type.key.type.to_string(), keyVar))
+                print("        datum.keys = key;")
+                if type.key.type == StringType:
+                    print("        key->s = ovsdb_atom_string_create(" + 
keyVar + ");")
+                else:
+                    print("        " + 
type.key.assign_c_value_casting_away_const("key->%s" % 
type.key.type.to_string(), keyVar))
                 print("    } else {")
                 print("        datum.n = 0;")
                 print("        datum.keys = NULL;")
                 print("    }")
                 print("    datum.values = NULL;")
-                txn_write_func = "ovsdb_idl_txn_write_clone"
+                txn_write_func = "ovsdb_idl_txn_write"
             elif type.n_max == 1:
-                print("    union ovsdb_atom key;")
                 print("")
                 print("    if (%s) {" % nVar)
+                print("        union ovsdb_atom *key = xmalloc(sizeof *key);")
                 print("        datum.n = 1;")
-                print("        datum.keys = &key;")
-                print("        " + 
type.key.assign_c_value_casting_away_const("key.%s" % 
type.key.type.to_string(), "*" + keyVar))
+                print("        datum.keys = key;")
+                if type.key.type == StringType:
+                    print("        key->s = ovsdb_atom_string_create(*" + 
keyVar + ");")
+                else:
+                    print("        " + 
type.key.assign_c_value_casting_away_const("key->%s" % 
type.key.type.to_string(), "*" + keyVar))
                 print("    } else {")
                 print("        datum.n = 0;")
                 print("        datum.keys = NULL;")
                 print("    }")
                 print("    datum.values = NULL;")
-                txn_write_func = "ovsdb_idl_txn_write_clone"
+                txn_write_func = "ovsdb_idl_txn_write"
             else:
                 print("")
                 print("    datum.n = %s;" % nVar)
@@ -984,9 +996,15 @@ void
                 else:
                     print("    datum.values = NULL;")
                 print("    for (size_t i = 0; i < %s; i++) {" % nVar)
-                print("        " + type.key.copyCValue("datum.keys[i].%s" % 
type.key.type.to_string(), "%s[i]" % keyVar))
+                if type.key.type == StringType:
+                    print("        datum.keys[i].s = 
ovsdb_atom_string_create(" + keyVar + "[i]);")
+                else:
+                    print("        " + type.key.copyCValue("datum.keys[i].%s" 
% type.key.type.to_string(), "%s[i]" % keyVar))
                 if type.value:
-                    print("        " + 
type.value.copyCValue("datum.values[i].%s" % type.value.type.to_string(), 
"%s[i]" % valueVar))
+                    if type.value.type == StringType:
+                        print("        datum.values[i].s = 
ovsdb_atom_string_create(" + valueVar + "[i]);")
+                    else:
+                        print("        " + 
type.value.copyCValue("datum.values[i].%s" % type.value.type.to_string(), 
"%s[i]" % valueVar))
                 print("    }")
                 if type.value:
                     valueType = type.value.toAtomicType()
@@ -1022,9 +1040,14 @@ void
 ''' % {'s': structName, 'c': 
columnName,'coltype':column.type.key.to_const_c_type(prefix),
         'valtype':column.type.value.to_const_c_type(prefix), 'S': 
structName.upper(),
         'C': columnName.upper(), 't': tableName})
-
-                print("    "+ type.key.copyCValue("datum->keys[0].%s" % 
type.key.type.to_string(), "new_key"))
-                print("    "+ type.value.copyCValue("datum->values[0].%s" % 
type.value.type.to_string(), "new_value"))
+                if type.key.type == StringType:
+                    print("    datum->keys[0].s = 
ovsdb_atom_string_create(new_key);")
+                else:
+                    print("    "+ type.key.copyCValue("datum->keys[0].%s" % 
type.key.type.to_string(), "new_key"))
+                if type.value.type == StringType:
+                    print("    datum->values[0].s = 
ovsdb_atom_string_create(new_value);")
+                else:
+                    print("    "+ type.value.copyCValue("datum->values[0].%s" 
% type.value.type.to_string(), "new_value"))
                 print('''
     ovsdb_idl_txn_write_partial_map(&row->header_,
                                     &%(s)s_col_%(c)s,
@@ -1048,8 +1071,10 @@ void
 ''' % {'s': structName, 'c': 
columnName,'coltype':column.type.key.to_const_c_type(prefix),
         'valtype':column.type.value.to_const_c_type(prefix), 'S': 
structName.upper(),
         'C': columnName.upper(), 't': tableName})
-
-                print("    "+ type.key.copyCValue("datum->keys[0].%s" % 
type.key.type.to_string(), "delete_key"))
+                if type.key.type == StringType:
+                    print("    datum->keys[0].s = 
ovsdb_atom_string_create(delete_key);")
+                else:
+                    print("    "+ type.key.copyCValue("datum->keys[0].%s" % 
type.key.type.to_string(), "delete_key"))
                 print('''
     ovsdb_idl_txn_delete_partial_map(&row->header_,
                                     &%(s)s_col_%(c)s,
@@ -1075,8 +1100,10 @@ void
     datum->values = NULL;
 ''' % {'s': structName, 'c': columnName,
         'valtype':column.type.key.to_const_c_type(prefix), 't': tableName})
-
-                print("    "+ type.key.copyCValue("datum->keys[0].%s" % 
type.key.type.to_string(), "new_value"))
+                if type.key.type == StringType:
+                    print("    datum->keys[0].s = 
ovsdb_atom_string_create(new_value);")
+                else:
+                    print("    "+ type.key.copyCValue("datum->keys[0].%s" % 
type.key.type.to_string(), "new_value"))
                 print('''
     ovsdb_idl_txn_write_partial_set(&row->header_,
                                     &%(s)s_col_%(c)s,
@@ -1100,8 +1127,10 @@ void
 ''' % {'s': structName, 'c': 
columnName,'coltype':column.type.key.to_const_c_type(prefix),
         'valtype':column.type.key.to_const_c_type(prefix), 'S': 
structName.upper(),
         'C': columnName.upper(), 't': tableName})
-
-                print("    "+ type.key.copyCValue("datum->keys[0].%s" % 
type.key.type.to_string(), "delete_value"))
+                if type.key.type == StringType:
+                    print("    datum->keys[0].s = 
ovsdb_atom_string_create(delete_value);")
+                else:
+                    print("    "+ type.key.copyCValue("datum->keys[0].%s" % 
type.key.type.to_string(), "delete_value"))
                 print('''
     ovsdb_idl_txn_delete_partial_set(&row->header_,
                                     &%(s)s_col_%(c)s,
@@ -1169,37 +1198,49 @@ void
             print("    struct ovsdb_datum datum;")
             free = []
             if type.n_min == 1 and type.n_max == 1:
-                print("    union ovsdb_atom key;")
+                print("    union ovsdb_atom *key = xmalloc(sizeof *key);")
                 if type.value:
-                    print("    union ovsdb_atom value;")
+                    print("    union ovsdb_atom *value = xmalloc(sizeof 
*value);")
                 print("")
                 print("    datum.n = 1;")
-                print("    datum.keys = &key;")
-                print("    " + 
type.key.assign_c_value_casting_away_const("key.%s" % 
type.key.type.to_string(), keyVar, refTable=False))
+                print("    datum.keys = key;")
+                if type.key.type == StringType:
+                    print("    key->s = ovsdb_atom_string_create(" + keyVar + 
");")
+                else:
+                    print("    " + 
type.key.assign_c_value_casting_away_const("key->%s" % 
type.key.type.to_string(), keyVar, refTable=False))
                 if type.value:
-                    print("    datum.values = &value;")
-                    print("    "+ 
type.value.assign_c_value_casting_away_const("value.%s" % 
type.value.type.to_string(), valueVar, refTable=False))
+                    print("    datum.values = value;")
+                    if type.value.type == StringType:
+                        print("    value->s = ovsdb_atom_string_create(" + 
valueVar + ");")
+                    else:
+                        print("    "+ 
type.value.assign_c_value_casting_away_const("value.%s" % 
type.value.type.to_string(), valueVar, refTable=False))
                 else:
                     print("    datum.values = NULL;")
             elif type.is_optional_pointer():
-                print("    union ovsdb_atom key;")
                 print("")
                 print("    if (%s) {" % keyVar)
+                print("        union ovsdb_atom *key = xmalloc(sizeof *key);")
                 print("        datum.n = 1;")
-                print("        datum.keys = &key;")
-                print("        " + 
type.key.assign_c_value_casting_away_const("key.%s" % 
type.key.type.to_string(), keyVar, refTable=False))
+                print("        datum.keys = key;")
+                if type.key.type == StringType:
+                    print("        key->s = ovsdb_atom_string_create(" + 
keyVar + ");")
+                else:
+                    print("        " + 
type.key.assign_c_value_casting_away_const("key->%s" % 
type.key.type.to_string(), keyVar, refTable=False))
                 print("    } else {")
                 print("        datum.n = 0;")
                 print("        datum.keys = NULL;")
                 print("    }")
                 print("    datum.values = NULL;")
             elif type.n_max == 1:
-                print("    union ovsdb_atom key;")
                 print("")
                 print("    if (%s) {" % nVar)
+                print("        union ovsdb_atom *key = xmalloc(sizeof *key);")
                 print("        datum.n = 1;")
-                print("        datum.keys = &key;")
-                print("        " + 
type.key.assign_c_value_casting_away_const("key.%s" % 
type.key.type.to_string(), "*" + keyVar, refTable=False))
+                print("        datum.keys = key;")
+                if type.key.type == StringType:
+                    print("        key->s = ovsdb_atom_string_create(*" + 
keyVar + ");")
+                else:
+                    print("        " + 
type.key.assign_c_value_casting_away_const("key->%s" % 
type.key.type.to_string(), "*" + keyVar, refTable=False))
                 print("    } else {")
                 print("        datum.n = 0;")
                 print("        datum.keys = NULL;")
@@ -1208,16 +1249,20 @@ void
             else:
                 print("    datum.n = %s;" % nVar)
                 print("    datum.keys = %s ? xmalloc(%s * sizeof *datum.keys) 
: NULL;" % (nVar, nVar))
-                free += ['datum.keys']
                 if type.value:
                     print("    datum.values = xmalloc(%s * sizeof 
*datum.values);" % nVar)
-                    free += ['datum.values']
                 else:
                     print("    datum.values = NULL;")
                 print("    for (size_t i = 0; i < %s; i++) {" % nVar)
-                print("        " + 
type.key.assign_c_value_casting_away_const("datum.keys[i].%s" % 
type.key.type.to_string(), "%s[i]" % keyVar, refTable=False))
+                if type.key.type == StringType:
+                    print("        datum.keys[i].s = 
ovsdb_atom_string_create(" + keyVar + "[i]);")
+                else:
+                    print("        " + 
type.key.assign_c_value_casting_away_const("datum.keys[i].%s" % 
type.key.type.to_string(), "%s[i]" % keyVar, refTable=False))
                 if type.value:
-                    print("        " + 
type.value.assign_c_value_casting_away_const("datum.values[i].%s" % 
type.value.type.to_string(), "%s[i]" % valueVar, refTable=False))
+                    if type.value.type == StringType:
+                        print("        datum.values[i].s = 
ovsdb_atom_string_create(" + valueVar + "[i]);")
+                    else:
+                        print("        " + 
type.value.assign_c_value_casting_away_const("datum.values[i].%s" % 
type.value.type.to_string(), "%s[i]" % valueVar, refTable=False))
                 print("    }")
                 if type.value:
                     valueType = type.value.toAtomicType()
@@ -1237,8 +1282,8 @@ void
        's': structName,
        'S': structName.upper(),
        'c': columnName})
-            for var in free:
-                print("    free(%s);" % var)
+            print("    ovsdb_datum_destroy(&datum, &%(s)s_col_%(c)s.type);" \
+                  % {'s': structName, 'c': columnName})
             print("}")
 
 # Index table related functions
@@ -1335,8 +1380,8 @@ struct %(s)s *
 
         i = 0;
         SMAP_FOR_EACH (node, %(c)s) {
-            datum->keys[i].string = node->key;
-            datum->values[i].string = node->value;
+            datum->keys[i].s = ovsdb_atom_string_create(node->key);
+            datum->values[i].s = ovsdb_atom_string_create(node->value);
             i++;
         }
         ovsdb_datum_sort_unique(datum, OVSDB_TYPE_STRING, OVSDB_TYPE_STRING);
@@ -1385,10 +1430,16 @@ struct %(s)s *
                 print()
                 print("    datum.n = 1;")
                 print("    datum.keys = key;")
-                print("    " + 
type.key.assign_c_value_casting_away_const("key->%s" % 
type.key.type.to_string(), keyVar))
+                if type.key.type == StringType:
+                    print("    key->s = ovsdb_atom_string_create(" + keyVar + 
");")
+                else:
+                    print("    " + 
type.key.assign_c_value_casting_away_const("key->%s" % 
type.key.type.to_string(), keyVar))
                 if type.value:
                     print("    datum.values = value;")
-                    print("    "+ 
type.value.assign_c_value_casting_away_const("value->%s" % 
type.value.type.to_string(), valueVar))
+                    if type.value.type == StringType:
+                        print("    value->s = ovsdb_atom_string_create(" + 
valueVar + ");")
+                    else:
+                        print("    " + 
type.value.assign_c_value_casting_away_const("value->%s" % 
type.value.type.to_string(), valueVar))
                 else:
                     print("    datum.values = NULL;")
                 txn_write_func = "ovsdb_idl_index_write"
@@ -1399,7 +1450,10 @@ struct %(s)s *
                 print("        key = xmalloc(sizeof (union ovsdb_atom));")
                 print("        datum.n = 1;")
                 print("        datum.keys = key;")
-                print("        " + 
type.key.assign_c_value_casting_away_const("key->%s" % 
type.key.type.to_string(), keyVar))
+                if type.key.type == StringType:
+                    print("        key->s = ovsdb_atom_string_create(" + 
keyVar + ");")
+                else:
+                    print("        " + 
type.key.assign_c_value_casting_away_const("key->%s" % 
type.key.type.to_string(), keyVar))
                 print("    } else {")
                 print("        datum.n = 0;")
                 print("        datum.keys = NULL;")
@@ -1413,7 +1467,10 @@ struct %(s)s *
                 print("        key = xmalloc(sizeof(union ovsdb_atom));")
                 print("        datum.n = 1;")
                 print("        datum.keys = key;")
-                print("        " + 
type.key.assign_c_value_casting_away_const("key->%s" % 
type.key.type.to_string(), "*" + keyVar))
+                if type.key.type == StringType:
+                    print("        key->s = ovsdb_atom_string_create(*" + 
keyVar + ");")
+                else:
+                    print("        " + 
type.key.assign_c_value_casting_away_const("key->%s" % 
type.key.type.to_string(), "*" + keyVar))
                 print("    } else {")
                 print("        datum.n = 0;")
                 print("        datum.keys = NULL;")
@@ -1430,9 +1487,15 @@ struct %(s)s *
                 else:
                     print("    datum.values = NULL;")
                 print("    for (i = 0; i < %s; i++) {" % nVar)
-                print("        " + type.key.copyCValue("datum.keys[i].%s" % 
type.key.type.to_string(), "%s[i]" % keyVar))
+                if type.key.type == StringType:
+                    print("        datum.keys[i].s = 
ovsdb_atom_string_create(" + keyVar + "[i]);")
+                else:
+                    print("        " + type.key.copyCValue("datum.keys[i].%s" 
% type.key.type.to_string(), "%s[i]" % keyVar))
                 if type.value:
-                    print("        " + 
type.value.copyCValue("datum.values[i].%s" % type.value.type.to_string(), 
"%s[i]" % valueVar))
+                    if type.value.type == StringType:
+                        print("    datum.values[i].s = 
ovsdb_atom_string_create(" + valueVar + "[i]);")
+                    else:
+                        print("        " + 
type.value.copyCValue("datum.values[i].%s" % type.value.type.to_string(), 
"%s[i]" % valueVar))
                 print("    }")
                 if type.value:
                     valueType = type.value.toAtomicType()
diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index 0b3d2bb71..b34d97e29 100644
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -904,8 +904,8 @@ query_db_string(const struct shash *all_dbs, const char 
*name,
 
             datum = &row->fields[column->index];
             for (i = 0; i < datum->n; i++) {
-                if (datum->keys[i].string[0]) {
-                    return datum->keys[i].string;
+                if (datum->keys[i].s->string[0]) {
+                    return datum->keys[i].s->string;
                 }
             }
         }
@@ -1018,7 +1018,7 @@ query_db_remotes(const char *name, const struct shash 
*all_dbs,
 
             datum = &row->fields[column->index];
             for (i = 0; i < datum->n; i++) {
-                add_remote(remotes, datum->keys[i].string);
+                add_remote(remotes, datum->keys[i].s->string);
             }
         }
     } else if (column->type.key.type == OVSDB_TYPE_UUID
diff --git a/ovsdb/ovsdb-util.c b/ovsdb/ovsdb-util.c
index c4075cdae..6d7be066b 100644
--- a/ovsdb/ovsdb-util.c
+++ b/ovsdb/ovsdb-util.c
@@ -111,13 +111,13 @@ ovsdb_util_read_map_string_column(const struct ovsdb_row 
*row,
 
     for (i = 0; i < datum->n; i++) {
         atom_key = &datum->keys[i];
-        if (!strcmp(atom_key->string, key)) {
+        if (!strcmp(atom_key->s->string, key)) {
             atom_value = &datum->values[i];
             break;
         }
     }
 
-    return atom_value ? atom_value->string : NULL;
+    return atom_value ? atom_value->s->string : NULL;
 }
 
 /* Read string-uuid key-values from a map.  Returns the row associated with
@@ -143,7 +143,7 @@ ovsdb_util_read_map_string_uuid_column(const struct 
ovsdb_row *row,
     const struct ovsdb_datum *datum = &row->fields[column->index];
     for (size_t i = 0; i < datum->n; i++) {
         union ovsdb_atom *atom_key = &datum->keys[i];
-        if (!strcmp(atom_key->string, key)) {
+        if (!strcmp(atom_key->s->string, key)) {
             const union ovsdb_atom *atom_value = &datum->values[i];
             return ovsdb_table_get_row(ref_table, &atom_value->uuid);
         }
@@ -181,7 +181,7 @@ ovsdb_util_read_string_column(const struct ovsdb_row *row,
     const union ovsdb_atom *atom;
 
     atom = ovsdb_util_read_column(row, column_name, OVSDB_TYPE_STRING);
-    *stringp = atom ? atom->string : NULL;
+    *stringp = atom ? atom->s->string : NULL;
     return atom != NULL;
 }
 
@@ -269,8 +269,10 @@ ovsdb_util_write_string_column(struct ovsdb_row *row, 
const char *column_name,
                                const char *string)
 {
     if (string) {
-        const union ovsdb_atom atom = { .string = CONST_CAST(char *, string) };
+        union ovsdb_atom atom = {
+            .s = ovsdb_atom_string_create(CONST_CAST(char *, string)) };
         ovsdb_util_write_singleton(row, column_name, &atom, OVSDB_TYPE_STRING);
+        ovsdb_atom_destroy(&atom, OVSDB_TYPE_STRING);
     } else {
         ovsdb_util_clear_column(row, column_name);
     }
@@ -305,8 +307,8 @@ ovsdb_util_write_string_string_column(struct ovsdb_row *row,
     datum->values = xmalloc(n * sizeof *datum->values);
 
     for (i = 0; i < n; ++i) {
-        datum->keys[i].string = keys[i];
-        datum->values[i].string = values[i];
+        datum->keys[i].s = ovsdb_atom_string_create_nocopy(keys[i]);
+        datum->values[i].s = ovsdb_atom_string_create_nocopy(values[i]);
     }
 
     /* Sort and check constraints. */
diff --git a/ovsdb/rbac.c b/ovsdb/rbac.c
index 2986027c9..ff411675f 100644
--- a/ovsdb/rbac.c
+++ b/ovsdb/rbac.c
@@ -53,8 +53,8 @@ ovsdb_find_row_by_string_key(const struct ovsdb_table *table,
         HMAP_FOR_EACH (row, hmap_node, &table->rows) {
             const struct ovsdb_datum *datum = &row->fields[column->index];
             for (size_t i = 0; i < datum->n; i++) {
-                if (datum->keys[i].string[0] &&
-                    !strcmp(key, datum->keys[i].string)) {
+                if (datum->keys[i].s->string[0] &&
+                    !strcmp(key, datum->keys[i].s->string)) {
                     return row;
                 }
             }
@@ -113,7 +113,7 @@ ovsdb_rbac_authorized(const struct ovsdb_row *perms,
     }
 
     for (i = 0; i < datum->n; i++) {
-        const char *name = datum->keys[i].string;
+        const char *name = datum->keys[i].s->string;
         const char *value = NULL;
         bool is_map;
 
@@ -271,7 +271,7 @@ rbac_column_modification_permitted(const struct 
ovsdb_column *column,
     size_t i;
 
     for (i = 0; i < modifiable->n; i++) {
-        char *name = modifiable->keys[i].string;
+        char *name = modifiable->keys[i].s->string;
 
         if (!strcmp(name, column->name)) {
             return true;
diff --git a/python/ovs/db/data.py b/python/ovs/db/data.py
index 2a2102d6b..99bf80ed6 100644
--- a/python/ovs/db/data.py
+++ b/python/ovs/db/data.py
@@ -204,7 +204,7 @@ class Atom(object):
             else:
                 return '.boolean = false'
         elif self.type == ovs.db.types.StringType:
-            return '.string = "%s"' % escapeCString(self.value)
+            return '.s = %s' % escapeCString(self.value)
         elif self.type == ovs.db.types.UuidType:
             return '.uuid = %s' % ovs.ovsuuid.to_c_assignment(self.value)
 
@@ -563,16 +563,41 @@ class Datum(object):
         if n == 0:
             return ["static struct ovsdb_datum %s = { .n = 0 };"]
 
-        s = ["static union ovsdb_atom %s_keys[%d] = {" % (name, n)]
-        for key in sorted(self.values):
-            s += ["    { %s }," % key.cInitAtom(key)]
-        s += ["};"]
+        s = []
+        if self.type.key.type == ovs.db.types.StringType:
+            s += ["static struct ovsdb_atom_string %s_key_strings[%d] = {"
+                  % (name, n)]
+            for key in sorted(self.values):
+                s += ['    { .string = "%s", .n_refs = 2 },'
+                      % escapeCString(key.value)]
+            s += ["};"]
+            s += ["static union ovsdb_atom %s_keys[%d] = {" % (name, n)]
+            for i in range(n):
+                s += ["    { .s = &%s_key_strings[%d] }," % (name, i)]
+            s += ["};"]
+        else:
+            s = ["static union ovsdb_atom %s_keys[%d] = {" % (name, n)]
+            for key in sorted(self.values):
+                s += ["    { %s }," % key.cInitAtom(key)]
+            s += ["};"]
 
         if self.type.value:
-            s = ["static union ovsdb_atom %s_values[%d] = {" % (name, n)]
-            for k, v in sorted(self.values.items()):
-                s += ["    { %s }," % v.cInitAtom(v)]
-            s += ["};"]
+            if self.type.value.type == ovs.db.types.StringType:
+                s += ["static struct ovsdb_atom_string %s_val_strings[%d] = {"
+                      % (name, n)]
+                for k, v in sorted(self.values):
+                    s += ['    { .string = "%s", .n_refs = 2 },'
+                          % escapeCString(v.value)]
+                s += ["};"]
+                s += ["static union ovsdb_atom %s_values[%d] = {" % (name, n)]
+                for i in range(n):
+                    s += ["    { .s = &%s_val_strings[%d] }," % (name, i)]
+                s += ["};"]
+            else:
+                s = ["static union ovsdb_atom %s_values[%d] = {" % (name, n)]
+                for k, v in sorted(self.values.items()):
+                    s += ["    { %s }," % v.cInitAtom(v)]
+                s += ["};"]
 
         s += ["static struct ovsdb_datum %s = {" % name]
         s += ["    .n = %d," % n]
diff --git a/python/ovs/db/types.py b/python/ovs/db/types.py
index 626ae8fc4..18485f701 100644
--- a/python/ovs/db/types.py
+++ b/python/ovs/db/types.py
@@ -48,6 +48,11 @@ class AtomicType(object):
     def to_string(self):
         return self.name
 
+    def to_Cvalue_string(self):
+        if self == StringType:
+            return 's->' + self.name
+        return self.name
+
     def to_json(self):
         return self.name
 
@@ -373,7 +378,7 @@ class BaseType(object):
                 return "%(dst)s = *%(src)s;" % args
             return ("%(dst)s = %(src)s->header_.uuid;") % args
         elif self.type == StringType:
-            return "%(dst)s = xstrdup(%(src)s);" % args
+            return "%(dst)s = ovsdb_atom_string_create(%(src)s);" % args
         else:
             return "%(dst)s = %(src)s;" % args
 
diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index 02485b136..c3ce4f361 100644
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -2727,13 +2727,15 @@ print_idl_row_simple2(const struct idltest_simple2 *s, 
int step)
     printf("%03d: name=%s smap=[",
            step, s->name);
     for (i = 0; i < smap->n; i++) {
-        printf("[%s : %s]%s", smap->keys[i].string, smap->values[i].string,
-                i < smap->n-1? ",": "");
+        printf("[%s : %s]%s",
+               smap->keys[i].s->string, smap->values[i].s->string,
+               i < smap->n - 1 ? "," : "");
     }
     printf("] imap=[");
     for (i = 0; i < imap->n; i++) {
-        printf("[%"PRId64" : %s]%s", imap->keys[i].integer, 
imap->values[i].string,
-                i < imap->n-1? ",":"");
+        printf("[%"PRId64" : %s]%s",
+               imap->keys[i].integer, imap->values[i].s->string,
+               i < imap->n - 1 ? "," : "");
     }
     printf("]\n");
 }
@@ -2802,8 +2804,8 @@ do_idl_partial_update_map_column(struct ovs_cmdl_context 
*ctx)
     myTxn = ovsdb_idl_txn_create(idl);
     smap = idltest_simple2_get_smap(myRow, OVSDB_TYPE_STRING,
                                     OVSDB_TYPE_STRING);
-    strcpy(key_to_delete, smap->keys[0].string);
-    idltest_simple2_update_smap_delkey(myRow, smap->keys[0].string);
+    ovs_strlcpy(key_to_delete, smap->keys[0].s->string, sizeof key_to_delete);
+    idltest_simple2_update_smap_delkey(myRow, smap->keys[0].s->string);
     ovsdb_idl_txn_commit_block(myTxn);
     ovsdb_idl_txn_destroy(myTxn);
     ovsdb_idl_get_initial_snapshot(idl);
-- 
2.31.1


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

Reply via email to