[ovs-dev] [PATCH 01/15] ovsdb-idlc: Use ovsdb_datum_from_smap() instead of open-coding it.

Ben Pfaff blp at ovn.org
Thu Oct 6 03:16:38 UTC 2016


There's no reason to have three copies of this code for every smap-type
column.

The code wasn't a perfect match for ovsdb_datum_from_smap(), so this commit
also changes ovsdb_datum_from_smap() to better suit it.  It only had one
caller and the new design is adequate for that caller.

Signed-off-by: Ben Pfaff <blp at ovn.org>
---
 lib/ovsdb-data.c    | 21 +++++++++------------
 lib/ovsdb-data.h    |  4 ++--
 ovsdb/ovsdb-idlc.in | 45 +++------------------------------------------
 vswitchd/bridge.c   |  1 +
 4 files changed, 15 insertions(+), 56 deletions(-)

diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c
index 266a3e4..0dda73a 100644
--- a/lib/ovsdb-data.c
+++ b/lib/ovsdb-data.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011, 2012, 2014 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012, 2014, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -1542,27 +1542,24 @@ ovsdb_datum_to_bare(const struct ovsdb_datum *datum,
     }
 }
 
-/* Initializes 'datum' as a string-to-string map whose contents are taken from
- * 'smap'.  Destroys 'smap'. */
+/* Initializes 'datum' as a string-to-string map whose contents are copied from
+ * 'smap', which is not modified. */
 void
-ovsdb_datum_from_smap(struct ovsdb_datum *datum, struct smap *smap)
+ovsdb_datum_from_smap(struct ovsdb_datum *datum, const struct smap *smap)
 {
-    struct smap_node *node, *next;
-    size_t i;
-
     datum->n = smap_count(smap);
     datum->keys = xmalloc(datum->n * sizeof *datum->keys);
     datum->values = xmalloc(datum->n * sizeof *datum->values);
 
-    i = 0;
-    SMAP_FOR_EACH_SAFE (node, next, smap) {
-        smap_steal(smap, node,
-                   &datum->keys[i].string, &datum->values[i].string);
+    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);
         i++;
     }
     ovs_assert(i == datum->n);
 
-    smap_destroy(smap);
     ovsdb_datum_sort_unique(datum, OVSDB_TYPE_STRING, OVSDB_TYPE_STRING);
 }
 
diff --git a/lib/ovsdb-data.h b/lib/ovsdb-data.h
index 98633ef..ae2672e 100644
--- a/lib/ovsdb-data.h
+++ b/lib/ovsdb-data.h
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011, 2012, 2015 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012, 2015, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -178,7 +178,7 @@ void ovsdb_datum_to_string(const struct ovsdb_datum *,
 void ovsdb_datum_to_bare(const struct ovsdb_datum *,
                          const struct ovsdb_type *, struct ds *);
 
-void ovsdb_datum_from_smap(struct ovsdb_datum *, struct smap *);
+void ovsdb_datum_from_smap(struct ovsdb_datum *, const struct smap *);
 
 /* Comparison. */
 uint32_t ovsdb_datum_hash(const struct ovsdb_datum *,
diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
index 0037b90..fb2241b 100755
--- a/ovsdb/ovsdb-idlc.in
+++ b/ovsdb/ovsdb-idlc.in
@@ -678,20 +678,7 @@ const struct ovsdb_datum *
 
     ovs_assert(inited);
     if (%(c)s) {
-        struct smap_node *node;
-        size_t i;
-
-        datum.n = smap_count(%(c)s);
-        datum.keys = xmalloc(datum.n * sizeof *datum.keys);
-        datum.values = xmalloc(datum.n * sizeof *datum.values);
-
-        i = 0;
-        SMAP_FOR_EACH (node, %(c)s) {
-            datum.keys[i].string = xstrdup(node->key);
-            datum.values[i].string = xstrdup(node->value);
-            i++;
-        }
-        ovsdb_datum_sort_unique(&datum, OVSDB_TYPE_STRING, OVSDB_TYPE_STRING);
+        ovsdb_datum_from_smap(&datum, %(c)s);
     } else {
         ovsdb_datum_init_empty(&datum);
     }
@@ -932,20 +919,7 @@ void
 
     ovs_assert(inited);
     if (%(c)s) {
-        struct smap_node *node;
-        size_t i;
-
-        datum.n = smap_count(%(c)s);
-        datum.keys = xmalloc(datum.n * sizeof *datum.keys);
-        datum.values = xmalloc(datum.n * sizeof *datum.values);
-
-        i = 0;
-        SMAP_FOR_EACH (node, %(c)s) {
-            datum.keys[i].string = xstrdup(node->key);
-            datum.values[i].string = xstrdup(node->value);
-            i++;
-        }
-        ovsdb_datum_sort_unique(&datum, OVSDB_TYPE_STRING, OVSDB_TYPE_STRING);
+        ovsdb_datum_from_smap(&datum, %(c)s);
     } else {
         ovsdb_datum_init_empty(&datum);
     }
@@ -1107,20 +1081,7 @@ void
 
     ovs_assert(inited);
     if (%(c)s) {
-        struct smap_node *node;
-        size_t i;
-
-        datum.n = smap_count(%(c)s);
-        datum.keys = xmalloc(datum.n * sizeof *datum.keys);
-        datum.values = xmalloc(datum.n * sizeof *datum.values);
-
-        i = 0;
-        SMAP_FOR_EACH (node, %(c)s) {
-            datum.keys[i].string = xstrdup(node->key);
-            datum.values[i].string = xstrdup(node->value);
-            i++;
-        }
-        ovsdb_datum_sort_unique(&datum, OVSDB_TYPE_STRING, OVSDB_TYPE_STRING);
+        ovsdb_datum_from_smap(&datum, %(c)s);
     } else {
         ovsdb_datum_init_empty(&datum);
     }
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 61cb966..4504217 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -2618,6 +2618,7 @@ run_system_stats(void)
 
         txn = ovsdb_idl_txn_create(idl);
         ovsdb_datum_from_smap(&datum, stats);
+        smap_destroy(stats);
         ovsdb_idl_txn_write(&cfg->header_, &ovsrec_open_vswitch_col_statistics,
                             &datum);
         ovsdb_idl_txn_commit(txn);
-- 
2.1.3




More information about the dev mailing list