On Tue, Jul 7, 2015 at 8:08 PM, Andy Zhou <az...@nicira.com> wrote:

> Both get_table() and set_cloum() APIs are mostly used within db-ctl-base
> library. This patch makes both private to the library.
>
> Add a new ctl_set_colum() API for library client.
>
> The changes are cleanups. No functional changes.
>
> Signed-off-by: Andy Zhou <az...@nicira.com>
> ---
>  lib/db-ctl-base.c     | 20 ++++++++++++++++++--
>  lib/db-ctl-base.h     | 12 +++---------
>  utilities/ovs-vsctl.c |  4 ++--
>  3 files changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
> index 10884b4..4092f2b 100644
> --- a/lib/db-ctl-base.c
> +++ b/lib/db-ctl-base.c
> @@ -46,7 +46,16 @@ VLOG_DEFINE_THIS_MODULE(db_ctl_base);
>  struct ovsdb_idl *the_idl;
>  struct ovsdb_idl_txn *the_idl_txn;
>
> +/* Represents all tables in the schema.  User must define 'tables'
> + * in implementation and supply via clt_init().  The definition must end
> + * with an all-NULL entry. */
> +const struct ctl_table_class *tables;
> +
>  static struct shash all_commands = SHASH_INITIALIZER(&all_commands);
> +static const struct ctl_table_class *get_table(const char *table_name);
> +static void set_column(const struct ctl_table_class *,
> +                       const struct ovsdb_idl_row *, const char *,
> +                       struct ovsdb_symbol_table *);
>
>
>  static struct option *
> @@ -1990,7 +1999,7 @@ ctl_context_done(struct ctl_context *ctx,
>
>  /* Finds and returns the "struct ctl_table_class *" with 'table_name' by
>   * searching the 'tables'. */
> -const struct ctl_table_class *
> +static const struct ctl_table_class *
>  get_table(const char *table_name)
>  {
>      const struct ctl_table_class *table;
> @@ -2018,7 +2027,7 @@ get_table(const char *table_name)
>  }
>
>  /* Sets the column of 'row' in 'table'. */
> -void
> +static void
>  set_column(const struct ctl_table_class *table,
>             const struct ovsdb_idl_row *row, const char *arg,
>             struct ovsdb_symbol_table *symtab)
> @@ -2070,3 +2079,10 @@ set_column(const struct ctl_table_class *table,
>      free(key_string);
>      free(value_string);
>  }
>


I tried to separate private, public functions.  Do you also think it makes
sense to move
static function up to one of the private functions sections?



> +
> +void ctl_set_column(const char *table_name,
> +                    const struct ovsdb_idl_row *row, const char *arg,
> +                    struct ovsdb_symbol_table *symtab)
> +{
> +    set_column(get_table(table_name), row, arg, symtab);
> +}
> diff --git a/lib/db-ctl-base.h b/lib/db-ctl-base.h
> index f14d27f..42b2e4a 100644
> --- a/lib/db-ctl-base.h
> +++ b/lib/db-ctl-base.h
> @@ -245,14 +245,8 @@ struct ctl_table_class {
>      struct ctl_row_id row_ids[2];
>  };
>
> -/* Represents all tables in the schema.  User must define 'tables'
> - * in implementation.  And the definition must end with an all-NULL
> - * entry. */
> -extern const struct ctl_table_class tables[];
> -
>


We should also delete this:

diff --git a/lib/db-ctl-base.h b/lib/db-ctl-base.h
index f14d27f..6089180 100644
--- a/lib/db-ctl-base.h
+++ b/lib/db-ctl-base.h
@@ -44,7 +44,6 @@ struct table;
  *   additional commands implemented by user.  (See 'struct ctl_context'
for
  *   more info)
  *
- * - the 'tables[]' for each table in the schema.
  *
 */




> -const struct ctl_table_class *get_table(const char *table_name);
> -void set_column(const struct ctl_table_class *,
> -                const struct ovsdb_idl_row *, const char *arg,
> -                struct ovsdb_symbol_table *);
> +void ctl_set_column(const char *table_name,
> +                    const struct ovsdb_idl_row *, const char *arg,
> +                    struct ovsdb_symbol_table *);
>
>  #endif /* db-ctl-base.h */
> diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
> index c9af355..863bc73 100644
> --- a/utilities/ovs-vsctl.c
> +++ b/utilities/ovs-vsctl.c
> @@ -1556,8 +1556,8 @@ add_port(struct ctl_context *ctx,
>      }
>
>      for (i = 0; i < n_settings; i++) {
> -        set_column(get_table("Port"), &port->header_, settings[i],
> -                   ctx->symtab);
> +        ctl_set_column("Port", &port->header_, settings[i],
> +                       ctx->symtab);
>      }
>
>      bridge_insert_port((bridge->parent ? bridge->parent->br_cfg
> --
> 1.9.1
>
>
Also, I think we should combine patch 2 and 3 since this patch will break
the unittests.


Thanks,
Alex Wang,




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

Reply via email to