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