On Wed, Jul 16, 2014 at 04:26:04PM -0700, Gurucharan Shetty wrote:
> On Wed, Jul 16, 2014 at 3:05 PM, Ben Pfaff <b...@nicira.com> wrote:
> > 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));
> > + }
> Extra '}'. Gives a compilation error.
Oops. Sorry about that, I broke up one commit into several and forgot
to test each one separately.
Removed.
> > }
> > }
> > }
> > @@ -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);
> You can probably get rid of the above line.
Good spotting. I removed it.
> Acked-by: Gurucharan Shetty <gshe...@nicira.com>
Thanks, I'll apply this to master soon.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
