[ovs-dev] [sparse2] Avoid sparse error or warning for sizeof(_Bool).

Ethan Jackson ethan at nicira.com
Sat May 21 01:33:59 UTC 2011


Looks Good.

Ethan

On Thu, May 12, 2011 at 13:46, Ben Pfaff <blp at nicira.com> 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
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>



More information about the dev mailing list