[ovs-dev] [PATCH 3/5] Simplify ofproto_controller_info by using a struct smap in place of array.

[Ben Pfaff]

The only client for ofproto_controller_info was transforming the array of
pairs into an smap anyway.  It's easy for the code that fills in the array
to generate it as an smap directly, and it's also easier to extend later.

Signed-off-by: Ben Pfaff <b...@nicira.com>
---
 ofproto/connmgr.c |   27 ++++++++++-----------------
 ofproto/ofproto.h |    8 ++------
 vswitchd/bridge.c |   10 +---------
 3 files changed, 13 insertions(+), 32 deletions(-)

diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 41a58c9..c7deea3 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -477,28 +477,23 @@ connmgr_get_controller_info(struct connmgr *mgr, struct 
shash *info)
             cinfo->is_connected = rconn_is_connected(rconn);
             cinfo->role = ofconn->role;
 
-            cinfo->pairs.n = 0;
-
+            smap_init(&cinfo->pairs);
             if (last_error) {
-                cinfo->pairs.keys[cinfo->pairs.n] = "last_error";
-                cinfo->pairs.values[cinfo->pairs.n++]
-                    = xstrdup(ovs_retval_to_string(last_error));
+                smap_add(&cinfo->pairs, "last_error",
+                         ovs_retval_to_string(last_error));
             }
 
-            cinfo->pairs.keys[cinfo->pairs.n] = "state";
-            cinfo->pairs.values[cinfo->pairs.n++]
-                = xstrdup(rconn_get_state(rconn));
+            smap_add(&cinfo->pairs, "state", rconn_get_state(rconn));
 
             if (last_connection != TIME_MIN) {
-                cinfo->pairs.keys[cinfo->pairs.n] = "sec_since_connect";
-                cinfo->pairs.values[cinfo->pairs.n++]
-                    = xasprintf("%ld", (long int) (now - last_connection));
+                smap_add_format(&cinfo->pairs, "sec_since_connect",
+                                "%ld", (long int) (now - last_connection));
             }
 
             if (last_disconnect != TIME_MIN) {
-                cinfo->pairs.keys[cinfo->pairs.n] = "sec_since_disconnect";
-                cinfo->pairs.values[cinfo->pairs.n++]
-                    = xasprintf("%ld", (long int) (now - last_disconnect));
+                smap_add_format(&cinfo->pairs, "sec_since_disconnect",
+                                "%ld", (long int) (now - last_disconnect));
+            }
             }
         }
     }
@@ -511,9 +506,7 @@ connmgr_free_controller_info(struct shash *info)
 
     SHASH_FOR_EACH (node, info) {
         struct ofproto_controller_info *cinfo = node->data;
-        while (cinfo->pairs.n) {
-            free(CONST_CAST(char *, cinfo->pairs.values[--cinfo->pairs.n]));
-        }
+        smap_destroy(&cinfo->pairs);
         free(cinfo);
     }
     shash_destroy(info);
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index d601181..c71662e 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -27,6 +27,7 @@
 #include "flow.h"
 #include "meta-flow.h"
 #include "netflow.h"
+#include "smap.h"
 #include "sset.h"
 #include "stp.h"
 
@@ -43,7 +44,6 @@ struct ofport;
 struct ofproto;
 struct shash;
 struct simap;
-struct smap;
 
 /* Needed for the lock annotations. */
 extern struct ovs_mutex ofproto_mutex;
@@ -51,11 +51,7 @@ extern struct ovs_mutex ofproto_mutex;
 struct ofproto_controller_info {
     bool is_connected;
     enum ofp12_controller_role role;
-    struct {
-        const char *keys[4];
-        const char *values[4];
-        size_t n;
-    } pairs;
+    struct smap pairs;
 };
 
 struct ofproto_sflow_options {
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 25e3279..8125ea4 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -2270,19 +2270,11 @@ refresh_controller_status(void)
 
         if (cinfo) {
             struct smap smap = SMAP_INITIALIZER(&smap);
-            const char **values = cinfo->pairs.values;
-            const char **keys = cinfo->pairs.keys;
-            size_t i;
-
-            for (i = 0; i < cinfo->pairs.n; i++) {
-                smap_add(&smap, keys[i], values[i]);
-            }
 
             ovsrec_controller_set_is_connected(cfg, cinfo->is_connected);
             ovsrec_controller_set_role(cfg, ofp12_controller_role_to_str(
                                            cinfo->role));
-            ovsrec_controller_set_status(cfg, &smap);
-            smap_destroy(&smap);
+            ovsrec_controller_set_status(cfg, &cinfo->pairs);
         } else {
             ovsrec_controller_set_is_connected(cfg, false);
             ovsrec_controller_set_role(cfg, NULL);
-- 
1.7.10.4

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev