Simplify ofproto_controller_info by using a struct smap in place of array.

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 <blp@nicira.com>
Acked-by: Gurucharan Shetty <gshetty@nicira.com>

diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 41a58c9..cfea2ea 100644 (file)
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -477,28 +477,22 @@ 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 +505,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 (file)
--- 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..3f17490 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -2269,20 +2269,10 @@ refresh_controller_status(void)
             shash_find_data(&info, cfg->target);
 
         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);