[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