Looks Good. Ethan
On Thu, May 12, 2011 at 13:46, Ben Pfaff <[email protected]> wrote: > The sparse checker does not like taking sizeof(_Bool). Older versions of > sparse output a hard error ("error: cannot size expression"). Newer > versions output a warning ("warning: expression using sizeof bool"). This > commit avoids the problem by not using sizeof(_Bool) anywhere. > > The only place where OVS uses sizeof(_Bool) anyway is in code generated by > the OVSDB IDL. It generates it for populating "optional bool" columns in > the database, that is, columns that are allowed to contain 0 or 1 instances > of a bool. For these columns, it generates code that looks roughly like > this: > row->column = xmalloc(sizeof *row->column); > *row->column = value; > This commit changes these columns from type "bool *" to type "const bool *" > and changes the generated code to: > static const bool true_value = true; > static const bool false_value = false; > > row->column = value ? &true_value : &false_value; > which avoids the problem and saves a malloc() call at the same time. > > The idltest code had a column with a slightly different type ("0, 1, or > 2 bools") that didn't fit the revised pattern and is a fairly stupid type > anyhow, so I just changed it to "0 or 1 bools". > --- > ovsdb/ovsdb-idlc.in | 34 ++++++++++++++++++++++++++++++++-- > tests/idltest.ovsschema | 2 +- > tests/ovsdb-idl-py.at | 22 +++++++++++----------- > tests/ovsdb-idl.at | 28 ++++++++++++++-------------- > vswitchd/bridge.c | 2 +- > 5 files changed, 59 insertions(+), 29 deletions(-) > > diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in > index 2a4c67c..e5c8183 100755 > --- a/ovsdb/ovsdb-idlc.in > +++ b/ovsdb/ovsdb-idlc.in > @@ -27,6 +27,8 @@ def constify(cType, const): > > def cMembers(prefix, columnName, column, const): > type = column.type > + if is_optional_bool(type): > + const = True > if type.n_min == 1 and type.n_max == 1: > singleton = True > pointer = '' > @@ -165,6 +167,10 @@ def printEnum(members): > print " %s" % members[-1] > print "};" > > +def is_optional_bool(type): > + return (type.key.type == ovs.db.types.BooleanType and not type.value > + and type.n_min == 0 and type.n_max == 1) > + > def printCIDLSource(schemaFile): > schema = parseSchema(schemaFile) > prefix = schema.idlPrefix > @@ -215,7 +221,23 @@ static void > keyVar = "row->%s" % columnName > valueVar = None > > - if (type.n_min == 1 and type.n_max == 1) or > type.is_optional_pointer(): > + if is_optional_bool(type): > + # Special case for an optional bool. This is only here > because > + # sparse does not like the "normal" case below ("warning: > + # expression using sizeof bool"). > + print > + print " assert(inited);" > + print " if (datum->n >= 1) {" > + print " static const bool false_value = false;" > + print " static const bool true_value = true;" > + print > + print " row->n_%s = 1;" % columnName > + print " %s = datum->keys[0].boolean ? &true_value : > &false_value;" % keyVar > + print " } else {" > + print " row->n_%s = 0;" % columnName > + print " %s = NULL;" % keyVar > + print " }" > + elif (type.n_min == 1 and type.n_max == 1) or > type.is_optional_pointer(): > print > print " assert(inited);" > print " if (datum->n >= 1) {" > @@ -283,7 +305,15 @@ static void > # Unparse functions. > for columnName, column in sorted(table.columns.iteritems()): > type = column.type > - if (type.n_min != 1 or type.n_max != 1) and not > type.is_optional_pointer(): > + if (type.key.type == ovs.db.types.BooleanType and not type.value > + and type.n_min == 0 and type.n_max == 1): > + print ''' > +static void > +%(s)s_unparse_%(c)s(struct ovsdb_idl_row *row OVS_UNUSED) > +{ > + /* Nothing to do. */ > +}''' % {'s': structName, 'c': columnName} > + elif (type.n_min != 1 or type.n_max != 1) and not > type.is_optional_pointer(): > print ''' > static void > %(s)s_unparse_%(c)s(struct ovsdb_idl_row *row_) > diff --git a/tests/idltest.ovsschema b/tests/idltest.ovsschema > index 275a49e..1d073aa 100644 > --- a/tests/idltest.ovsschema > +++ b/tests/idltest.ovsschema > @@ -60,7 +60,7 @@ > "ba": { > "type": { > "key": "boolean", > - "max": "unlimited", > + "max": 1, > "min": 0 > } > }, > diff --git a/tests/ovsdb-idl-py.at b/tests/ovsdb-idl-py.at > index 5f7661b..8104385 100644 > --- a/tests/ovsdb-idl-py.at > +++ b/tests/ovsdb-idl-py.at > @@ -51,7 +51,7 @@ OVSDB_CHECK_IDL([simple idl, initially empty, various ops - > Python], > "u": ["uuid", "84f5c8f5-ac76-4dbc-a24f-8860eb407fc1"], > "ia": ["set", [1, 2, 3]], > "ra": ["set", [-0.5]], > - "ba": ["set", [true, false]], > + "ba": ["set", [true]], > "sa": ["set", ["abc", "def"]], > "ua": ["set", [["uuid", > "69443985-7806-45e2-b35f-574a04e720f9"], > ["uuid", > "aad11ef0-816a-4b01-93e6-03b8b4256b98"]]]}}, > @@ -93,27 +93,27 @@ OVSDB_CHECK_IDL([simple idl, initially empty, various ops > - Python], > [[000: empty > 001: > {"error":null,"result":[{"uuid":["uuid","<0>"]},{"uuid":["uuid","<1>"]}]} > 002: i=0 r=0 b=false s= u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> > -002: i=1 r=2 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[false true] > sa=[abc def] ua=[<4> <5>] uuid=<0> > +002: i=1 r=2 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[true] sa=[abc > def] ua=[<4> <5>] uuid=<0> > 003: {"error":null,"result":[{"count":2}]} > 004: i=0 r=0 b=true s= u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> > -004: i=1 r=2 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[false true] > sa=[abc def] ua=[<4> <5>] uuid=<0> > +004: i=1 r=2 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[true] sa=[abc > def] ua=[<4> <5>] uuid=<0> > 005: {"error":null,"result":[{"count":2}]} > 006: i=0 r=123.5 b=true s= u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> > -006: i=1 r=123.5 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[false > true] sa=[abc def] ua=[<4> <5>] uuid=<0> > +006: i=1 r=123.5 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[true] > sa=[abc def] ua=[<4> <5>] uuid=<0> > 007: {"error":null,"result":[{"uuid":["uuid","<6>"]}]} > 008: i=-1 r=125 b=false s= u=<2> ia=[1] ra=[1.5] ba=[false] sa=[] ua=[] > uuid=<6> > 008: i=0 r=123.5 b=true s= u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> > -008: i=1 r=123.5 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[false > true] sa=[abc def] ua=[<4> <5>] uuid=<0> > +008: i=1 r=123.5 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[true] > sa=[abc def] ua=[<4> <5>] uuid=<0> > 009: {"error":null,"result":[{"count":2}]} > 010: i=-1 r=125 b=false s=newstring u=<2> ia=[1] ra=[1.5] ba=[false] sa=[] > ua=[] uuid=<6> > 010: i=0 r=123.5 b=true s=newstring u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] > uuid=<1> > -010: i=1 r=123.5 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[false > true] sa=[abc def] ua=[<4> <5>] uuid=<0> > +010: i=1 r=123.5 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[true] > sa=[abc def] ua=[<4> <5>] uuid=<0> > 011: {"error":null,"result":[{"count":1}]} > 012: i=-1 r=125 b=false s=newstring u=<2> ia=[1] ra=[1.5] ba=[false] sa=[] > ua=[] uuid=<6> > -012: i=1 r=123.5 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[false > true] sa=[abc def] ua=[<4> <5>] uuid=<0> > +012: i=1 r=123.5 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[true] > sa=[abc def] ua=[<4> <5>] uuid=<0> > 013: reconnect > 014: i=-1 r=125 b=false s=newstring u=<2> ia=[1] ra=[1.5] ba=[false] sa=[] > ua=[] uuid=<6> > -014: i=1 r=123.5 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[false > true] sa=[abc def] ua=[<4> <5>] uuid=<0> > +014: i=1 r=123.5 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[true] > sa=[abc def] ua=[<4> <5>] uuid=<0> > 015: done > ]]) > > @@ -128,7 +128,7 @@ OVSDB_CHECK_IDL([simple idl, initially populated - > Python], > "u": ["uuid", "84f5c8f5-ac76-4dbc-a24f-8860eb407fc1"], > "ia": ["set", [1, 2, 3]], > "ra": ["set", [-0.5]], > - "ba": ["set", [true, false]], > + "ba": ["set", [true]], > "sa": ["set", ["abc", "def"]], > "ua": ["set", [["uuid", > "69443985-7806-45e2-b35f-574a04e720f9"], > ["uuid", > "aad11ef0-816a-4b01-93e6-03b8b4256b98"]]]}}, > @@ -141,9 +141,9 @@ OVSDB_CHECK_IDL([simple idl, initially populated - > Python], > "where": [], > "row": {"b": true}}]']], > [[000: i=0 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> > -000: i=1 r=2 b=true s=mystring u=<2> ia=[1 2 3] ra=[-0.5] ba=[false true] > sa=[abc def] ua=[<3> <4>] uuid=<5> > +000: i=1 r=2 b=true s=mystring u=<2> ia=[1 2 3] ra=[-0.5] ba=[true] sa=[abc > def] ua=[<3> <4>] uuid=<5> > 001: {"error":null,"result":[{"count":2}]} > 002: i=0 r=0 b=true s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> > -002: i=1 r=2 b=true s=mystring u=<2> ia=[1 2 3] ra=[-0.5] ba=[false true] > sa=[abc def] ua=[<3> <4>] uuid=<5> > +002: i=1 r=2 b=true s=mystring u=<2> ia=[1 2 3] ra=[-0.5] ba=[true] sa=[abc > def] ua=[<3> <4>] uuid=<5> > 003: done > ]]) > diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at > index 3b4cfc8..5956f72 100644 > --- a/tests/ovsdb-idl.at > +++ b/tests/ovsdb-idl.at > @@ -50,7 +50,7 @@ OVSDB_CHECK_IDL([simple idl, initially empty, various ops], > "u": ["uuid", "84f5c8f5-ac76-4dbc-a24f-8860eb407fc1"], > "ia": ["set", [1, 2, 3]], > "ra": ["set", [-0.5]], > - "ba": ["set", [true, false]], > + "ba": ["set", [true]], > "sa": ["set", ["abc", "def"]], > "ua": ["set", [["uuid", > "69443985-7806-45e2-b35f-574a04e720f9"], > ["uuid", > "aad11ef0-816a-4b01-93e6-03b8b4256b98"]]]}}, > @@ -92,27 +92,27 @@ OVSDB_CHECK_IDL([simple idl, initially empty, various > ops], > [[000: empty > 001: > {"error":null,"result":[{"uuid":["uuid","<0>"]},{"uuid":["uuid","<1>"]}]} > 002: i=0 r=0 b=false s= u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> > -002: i=1 r=2 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[false true] > sa=[abc def] ua=[<4> <5>] uuid=<0> > +002: i=1 r=2 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[true] sa=[abc > def] ua=[<4> <5>] uuid=<0> > 003: {"error":null,"result":[{"count":2}]} > 004: i=0 r=0 b=true s= u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> > -004: i=1 r=2 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[false true] > sa=[abc def] ua=[<4> <5>] uuid=<0> > +004: i=1 r=2 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[true] sa=[abc > def] ua=[<4> <5>] uuid=<0> > 005: {"error":null,"result":[{"count":2}]} > 006: i=0 r=123.5 b=true s= u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> > -006: i=1 r=123.5 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[false > true] sa=[abc def] ua=[<4> <5>] uuid=<0> > +006: i=1 r=123.5 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[true] > sa=[abc def] ua=[<4> <5>] uuid=<0> > 007: {"error":null,"result":[{"uuid":["uuid","<6>"]}]} > 008: i=-1 r=125 b=false s= u=<2> ia=[1] ra=[1.5] ba=[false] sa=[] ua=[] > uuid=<6> > 008: i=0 r=123.5 b=true s= u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> > -008: i=1 r=123.5 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[false > true] sa=[abc def] ua=[<4> <5>] uuid=<0> > +008: i=1 r=123.5 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[true] > sa=[abc def] ua=[<4> <5>] uuid=<0> > 009: {"error":null,"result":[{"count":2}]} > 010: i=-1 r=125 b=false s=newstring u=<2> ia=[1] ra=[1.5] ba=[false] sa=[] > ua=[] uuid=<6> > 010: i=0 r=123.5 b=true s=newstring u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] > uuid=<1> > -010: i=1 r=123.5 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[false > true] sa=[abc def] ua=[<4> <5>] uuid=<0> > +010: i=1 r=123.5 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[true] > sa=[abc def] ua=[<4> <5>] uuid=<0> > 011: {"error":null,"result":[{"count":1}]} > 012: i=-1 r=125 b=false s=newstring u=<2> ia=[1] ra=[1.5] ba=[false] sa=[] > ua=[] uuid=<6> > -012: i=1 r=123.5 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[false > true] sa=[abc def] ua=[<4> <5>] uuid=<0> > +012: i=1 r=123.5 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[true] > sa=[abc def] ua=[<4> <5>] uuid=<0> > 013: reconnect > 014: i=-1 r=125 b=false s=newstring u=<2> ia=[1] ra=[1.5] ba=[false] sa=[] > ua=[] uuid=<6> > -014: i=1 r=123.5 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[false > true] sa=[abc def] ua=[<4> <5>] uuid=<0> > +014: i=1 r=123.5 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[true] > sa=[abc def] ua=[<4> <5>] uuid=<0> > 015: done > ]]) > > @@ -127,7 +127,7 @@ OVSDB_CHECK_IDL([simple idl, initially populated], > "u": ["uuid", "84f5c8f5-ac76-4dbc-a24f-8860eb407fc1"], > "ia": ["set", [1, 2, 3]], > "ra": ["set", [-0.5]], > - "ba": ["set", [true, false]], > + "ba": ["set", [true]], > "sa": ["set", ["abc", "def"]], > "ua": ["set", [["uuid", > "69443985-7806-45e2-b35f-574a04e720f9"], > ["uuid", > "aad11ef0-816a-4b01-93e6-03b8b4256b98"]]]}}, > @@ -140,10 +140,10 @@ OVSDB_CHECK_IDL([simple idl, initially populated], > "where": [], > "row": {"b": true}}]']], > [[000: i=0 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> > -000: i=1 r=2 b=true s=mystring u=<2> ia=[1 2 3] ra=[-0.5] ba=[false true] > sa=[abc def] ua=[<3> <4>] uuid=<5> > +000: i=1 r=2 b=true s=mystring u=<2> ia=[1 2 3] ra=[-0.5] ba=[true] sa=[abc > def] ua=[<3> <4>] uuid=<5> > 001: {"error":null,"result":[{"count":2}]} > 002: i=0 r=0 b=true s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> > -002: i=1 r=2 b=true s=mystring u=<2> ia=[1 2 3] ra=[-0.5] ba=[false true] > sa=[abc def] ua=[<3> <4>] uuid=<5> > +002: i=1 r=2 b=true s=mystring u=<2> ia=[1 2 3] ra=[-0.5] ba=[true] sa=[abc > def] ua=[<3> <4>] uuid=<5> > 003: done > ]]) > > @@ -158,7 +158,7 @@ OVSDB_CHECK_IDL([simple idl, writing via IDL], > "u": ["uuid", "84f5c8f5-ac76-4dbc-a24f-8860eb407fc1"], > "ia": ["set", [1, 2, 3]], > "ra": ["set", [-0.5]], > - "ba": ["set", [true, false]], > + "ba": ["set", [true]], > "sa": ["set", ["abc", "def"]], > "ua": ["set", [["uuid", > "69443985-7806-45e2-b35f-574a04e720f9"], > ["uuid", > "aad11ef0-816a-4b01-93e6-03b8b4256b98"]]]}}, > @@ -168,10 +168,10 @@ OVSDB_CHECK_IDL([simple idl, writing via IDL], > [['verify 0 b, verify 1 r, set 0 b 1, set 1 r 3.5' \ > 'insert 2, verify 2 i, verify 1 b, delete 1']], > [[000: i=0 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> > -000: i=1 r=2 b=true s=mystring u=<2> ia=[1 2 3] ra=[-0.5] ba=[false true] > sa=[abc def] ua=[<3> <4>] uuid=<5> > +000: i=1 r=2 b=true s=mystring u=<2> ia=[1 2 3] ra=[-0.5] ba=[true] sa=[abc > def] ua=[<3> <4>] uuid=<5> > 001: commit, status=success > 002: i=0 r=0 b=true s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> > -002: i=1 r=3.5 b=true s=mystring u=<2> ia=[1 2 3] ra=[-0.5] ba=[false true] > sa=[abc def] ua=[<3> <4>] uuid=<5> > +002: i=1 r=3.5 b=true s=mystring u=<2> ia=[1 2 3] ra=[-0.5] ba=[true] > sa=[abc def] ua=[<3> <4>] uuid=<5> > 003: commit, status=success > 004: i=0 r=0 b=true s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> > 004: i=2 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<6> > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c > index 9accbdd..6b3c5e3 100644 > --- a/vswitchd/bridge.c > +++ b/vswitchd/bridge.c > @@ -1251,7 +1251,7 @@ iface_refresh_cfm_stats(struct iface *iface) > static bool > iface_refresh_lacp_stats(struct iface *iface) > { > - bool *db_current = iface->cfg->lacp_current; > + const bool *db_current = iface->cfg->lacp_current; > bool changed = false; > > if (iface->port->lacp) { > -- > 1.7.4.4 > > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
