On Tue, May 17, 2016 at 05:26:59PM +0300, Liran Schour wrote:
> Columns indexing is different in ovsdb_row then in ovsdb_monitor_row.
> We need mapping between the 2 for condition evaluation.
>
> signed-off-by: Liran Schour <lir...@il.ibm.com>
In ovsdb_monitor_table_columns_sort(), I don't understand why this loop
starts at 1 instead of 0:
> + qsort(mt->columns, mt->n_columns, sizeof *mt->columns,
> + compare_ovsdb_monitor_column);
> + for (i = 1; i < mt->n_columns; i++) {
> + /* re-set index map due to sort */
> + mt->columns_index_map[mt->columns[i].column->index] = i;
> + }
In ovsdb_monitor_add_table(), please use sizeof *mt->columns_index_map
instead of sizeof(unsigned int), following CodingStyle.md:
+ mt->columns_index_map =
+ xmalloc(sizeof(unsigned int) * shash_count(&table->schema->columns));
Also in ovsdb_monitor_add_table(), I think that the code would be a
little easier to read if shash_count(&table->schema->columns) were put
into a variables, e.g. n_columns.
This new columns_index_map means that it's no longer necessary to work
so hard to find duplicates--rather, we can find a duplicate at the time
we try to add a column: a new column is a duplicate if
columns_index_map[column->index] != -1 at the time we try to add it. It
would probably be nicer to simply report the error at the time we add
it, instead of adding a sort step and a re-indexing step.
Please add a period at the end of this comment, to make it look like a
complete thought:
+ /* Columns in ovsdb_monitor_row have different indexes then in
+ * ovsdb_row. This field maps between column->index to the index in the
+ * ovsdb_monitor_row. It is used for condition evaluation */
Thanks,
Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
