[ovs-dev] [PATCH 3/5] Simplify ofproto_controller_info by using a struct smap in place of array.
Gurucharan Shetty
shettyg at nicira.com
Wed Jul 16 23:26:04 UTC 2014
On Wed, Jul 16, 2014 at 3:05 PM, Ben Pfaff <blp at 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 <blp at 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.
> }
> }
> }
> @@ -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.
Acked-by: Gurucharan Shetty <gshetty at nicira.com>
More information about the dev
mailing list