On Thu, Oct 26, 2017 at 2:28 PM, Ben Pfaff <b...@ovn.org> wrote: > > On Sat, Oct 14, 2017 at 11:53:23PM -0700, Han Zhou wrote: > > IDL index should be able to be used without having to be in a > > transaction. However, current implementation leads to crash if > > a reference type column is being set in an index row for querying > > purpose when it is not in a transaction. It is because of the > > uninitialized arcs and unnecessary updates of the arcs. This patch > > fixes it by telling the column parsing function whether it is for > > index row or not, so that when parsing index row, the arcs are not > > updated. A new test case is added to cover this scenario. > > > > Signed-off-by: Han Zhou <zhou...@gmail.com> > > Thank you for the bug fix. I agree that this fix should go in. > > I see at least one place where "update" is misspelled "udpate". > > Did you consider whether there is a way to distinguish an index row from > other rows, without adding a new member? It might be possible to simply > consider the UUID, since an index row has UUID zero and other rows do > not. If this is possible, then I think I'd prefer that approach.
Thanks Ben. Yes, this is a better idea. I didn't want to add a new field in the row structure, but didn't thought about playing around with uuid. I am not sure if there is/will be any cases that a row that is not an index row could get parsed before the uuid is generated, so I think it is safer to use a magic UUID to distinguish index rows from others. I just submitted V2 for this. > > The following incremental fixes some style violations identified by > "checkpatch". > > --8<--------------------------cut here-------------------------->8-- > > diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c > index 3636a0e09e0e..451172cdcc34 100644 > --- a/tests/test-ovsdb.c > +++ b/tests/test-ovsdb.c > @@ -2683,8 +2683,8 @@ do_idl_compound_index_with_ref(struct ovs_cmdl_context *ctx) > > struct ovsdb_idl_index *index; > index = ovsdb_idl_create_index(idl, &idltest_table_simple3, "uref"); > - ovsdb_idl_index_add_column(index, &idltest_simple3_col_uref, OVSDB_INDEX_ASC, > - NULL); > + ovsdb_idl_index_add_column(index, &idltest_simple3_col_uref, > + OVSDB_INDEX_ASC, NULL); > > ovsdb_idl_get_initial_snapshot(idl); > > @@ -2706,7 +2706,7 @@ do_idl_compound_index_with_ref(struct ovs_cmdl_context *ctx) > printf("%03d: After add to other table + set of strong ref\n", step++); > dump_simple3(idl, myRow, step++); > > - myRow2 = (struct idltest_simple4*)idltest_simple4_first(idl); > + myRow2 = (struct idltest_simple4 *) idltest_simple4_first(idl); > printf("%03d: check simple4: %s\n", step++, > myRow2 ? "not empty" : "empty"); > > @@ -2714,7 +2714,7 @@ do_idl_compound_index_with_ref(struct ovs_cmdl_context *ctx) > > struct idltest_simple3 *equal; > equal = idltest_simple3_index_init_row(idl, &idltest_table_simple3); > - myRow2 = (struct idltest_simple4*)idltest_simple4_first(idl); > + myRow2 = (struct idltest_simple4 *) idltest_simple4_first(idl); > idltest_simple3_index_set_uref(equal, &myRow2, 1); > printf("%03d: Query using index with reference\n", step++); > IDLTEST_SIMPLE3_FOR_EACH_EQUAL (myRow, &cursor, equal) { > @@ -2732,7 +2732,7 @@ do_idl_compound_index_with_ref(struct ovs_cmdl_context *ctx) > printf("%03d: After delete\n", step++); > dump_simple3(idl, myRow, step++); > > - myRow2 = (struct idltest_simple4*)idltest_simple4_first(idl); > + myRow2 = (struct idltest_simple4 *) idltest_simple4_first(idl); > printf("%03d: check simple4: %s\n", step++, > myRow2 ? "not empty" : "empty"); > Now I know how to use checkpatch to make my life easier. Thanks! Han _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev