Looks good to me, thanks.

Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com>

On Sat, May 2, 2020 at 10:29 AM William Tu <u9012...@gmail.com> wrote:

> Coverity shows the following memory leak in this code pattern:
>
> void
> ovsrec_ipfix_index_set_obs_domain_id(
> {
>     struct ovsdb_datum datum;
> //  1. alloc_fn: Storage is returned from allocation function xmalloc.
> //  2. var_assign: Assigning: key = storage returned from xmalloc(16UL).
>     union ovsdb_atom *key = xmalloc(sizeof(union ovsdb_atom));
>
> //  3. Condition n_obs_domain_id, taking false branch.
>     if (n_obs_domain_id) {
>         datum.n = 1;
>         datum.keys = key;
>         key->integer = *obs_domain_id;
>     } else {
>         datum.n = 0;
>         datum.keys = NULL;
>     }
>     datum.values = NULL;
>     ovsdb_idl_index_write(CONST_CAST(struct ovsdb_idl_row *,
> //  CID 1420891 (#1 of 1): Resource leak (RESOURCE_LEAK)
>
> Fix it by moving the xmalloc to the true branch.
>
> Signed-off-by: William Tu <u9012...@gmail.com>
> ---
>  ovsdb/ovsdb-idlc.in | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
> index c285ee4b3c10..1d385e15c1e5 100755
> --- a/ovsdb/ovsdb-idlc.in
> +++ b/ovsdb/ovsdb-idlc.in
> @@ -1351,9 +1351,10 @@ struct %(s)s *
>                      print("    datum.values = NULL;")
>                  txn_write_func = "ovsdb_idl_index_write"
>              elif type.is_optional_pointer():
> -                print("    union ovsdb_atom *key = xmalloc(sizeof (union
> ovsdb_atom));")
> +                print("    union ovsdb_atom *key;")
>                  print()
>                  print("    if (%s) {" % keyVar)
> +                print("        key = xmalloc(sizeof (union ovsdb_atom));")
>                  print("        datum.n = 1;")
>                  print("        datum.keys = key;")
>                  print("        " +
> type.key.assign_c_value_casting_away_const("key->%s" %
> type.key.type.to_string(), keyVar))
> @@ -1364,9 +1365,10 @@ struct %(s)s *
>                  print("    datum.values = NULL;")
>                  txn_write_func = "ovsdb_idl_index_write"
>              elif type.n_max == 1:
> -                print("    union ovsdb_atom *key = xmalloc(sizeof(union
> ovsdb_atom));")
> +                print("    union ovsdb_atom *key;")
>                  print()
>                  print("    if (%s) {" % nVar)
> +                print("        key = xmalloc(sizeof(union ovsdb_atom));")
>                  print("        datum.n = 1;")
>                  print("        datum.keys = key;")
>                  print("        " +
> type.key.assign_c_value_casting_away_const("key->%s" %
> type.key.type.to_string(), "*" + keyVar))
> --
> 2.7.4
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to