bt2: fix: don't return in bt_bool out typemap
authorSimon Marchi <simon.marchi@efficios.com>
Thu, 18 Jul 2019 18:06:03 +0000 (14:06 -0400)
committerPhilippe Proulx <eeppeliteloop@gmail.com>
Sat, 20 Jul 2019 12:59:38 +0000 (08:59 -0400)
I noticed this leak in the Valgrind output:

    ==11115== 5 bytes in 1 blocks are definitely lost in loss record 9 of 5,534
    ==11115==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==11115==    by 0x8EA64A8: SWIG_AsCharPtrAndSize (native_bt.c:3539)
    ==11115==    by 0x8EDC722: _wrap_value_map_has_entry (native_bt.c:22039)
    ...

This is due to the fact that we return in the bt_bool out typemap, which
we shouldn't.  In the case of the bt_value_map_has_entry wrapper, a
string is dynamically allocated for the "const char *" parameter.  It is
normally freed at the end of the wrapper, except that because we return
early, it is never freed.  The same problem is likely to happen with
other functions returning bt_bool.

The leak can be triggered by running this Python script:

    from bt2 import value

    m = value.MapValue()
    m['allo'] = 2
    print('allo' in m)

The generated code before this patch looks like:

      result = (bt_bool)bt_value_map_has_entry((bt_value const *)arg1,(char const *)arg2);
      {
        if (result > 0) {
          resultobj = Py_True;
        } else {
          resultobj = Py_False;
        }
        Py_INCREF(resultobj);
        return resultobj;
      }
      if (alloc2 == SWIG_NEWOBJ) free((char*)buf2);
      return resultobj;
    fail:
      if (alloc2 == SWIG_NEWOBJ) free((char*)buf2);
      return NULL;
    }

The memory leak is quite obvious.  After this patch, it looks better:

      result = (bt_bool)bt_value_map_has_entry((bt_value const *)arg1,(char const *)arg2);
      {
        if (result > 0) {
          resultobj = Py_True;
        } else {
          resultobj = Py_False;
        }
        Py_INCREF(resultobj);
      }
      if (alloc2 == SWIG_NEWOBJ) free((char*)buf2);
      return resultobj;
    fail:
      if (alloc2 == SWIG_NEWOBJ) free((char*)buf2);
      return NULL;
    }

Change-Id: Icdfcdf750610465331619cd9edab1e89dc930f64
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1723
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
src/bindings/python/bt2/bt2/native_bt.i

index 49adfbfdf9b410fd6f719f48e5d58706c3bb9cdd..acf47b9daee1ecad2c2cd0ca7f40e82c457c983f 100644 (file)
@@ -164,7 +164,6 @@ typedef int bt_bool;
                $result = Py_False;
        }
        Py_INCREF($result);
-       return $result;
 }
 
 /*
This page took 0.02789 seconds and 4 git commands to generate.