From b0c88ede74392fc3d73270c3b9a545b7e641d9ab Mon Sep 17 00:00:00 2001 From: "E. Scott Daniels" Date: Thu, 13 Aug 2020 15:12:35 -0400 Subject: [PATCH] Correct potential leaks in xapp class. Sonar identified some potential memory leaks in the run function of the xapp class. This change addresses those issues, and ensures that the unused parameter grumblings which were attempted to be corrected in the last change are also addressed. Issue-ID: RIC-629 Signed-off-by: E. Scott Daniels Change-Id: I2fe2f4b681780cd60235a0c38ed4709af871abbe --- CHANGES | 3 +++ CMakeLists.txt | 2 +- docs/rel-notes.rst | 5 +++++ src/config/config.cpp | 3 +++ src/json/jwrapper.c | 28 ++++++++++++++-------------- src/xapp/xapp.cpp | 2 +- test/unit_test.sh | 7 +++++-- 7 files changed, 32 insertions(+), 18 deletions(-) diff --git a/CHANGES b/CHANGES index 5a9ef6c..864388f 100644 --- a/CHANGES +++ b/CHANGES @@ -6,6 +6,9 @@ # squished to one. release = Cherry +2020 13 August; version 2.2.2 + Correct potential memory leaks in xapp class (RIC-629) + 2020 July 31; version 2.2.1 Correct "bugs" according to sonar (RIC-629) diff --git a/CMakeLists.txt b/CMakeLists.txt index fe6467d..b4f7012 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -30,7 +30,7 @@ cmake_minimum_required( VERSION 3.5 ) set( major_version "2" ) # should be automatically populated from git tag later, but until CI process sets a tag we use this set( minor_version "2" ) -set( patch_level "1" ) +set( patch_level "2" ) set( install_root "${CMAKE_INSTALL_PREFIX}" ) set( install_inc "include/ricxfcpp" ) diff --git a/docs/rel-notes.rst b/docs/rel-notes.rst index 7f368e9..415d243 100644 --- a/docs/rel-notes.rst +++ b/docs/rel-notes.rst @@ -18,6 +18,11 @@ xAPP Framework. Cherry Release ============== +2020 13 August; version 2.2.2 +----------------------------- +Correct potential memory leaks in xapp class (RIC-629) + + 2020 July 31; version 2.2.1 --------------------------- Correct "bugs" according to sonar (RIC-629) diff --git a/src/config/config.cpp b/src/config/config.cpp index 16eb421..9cfa420 100644 --- a/src/config/config.cpp +++ b/src/config/config.cpp @@ -126,6 +126,9 @@ void xapp::Config::Listener( ) { } } } + + free( dname ); + free( bname ); } diff --git a/src/json/jwrapper.c b/src/json/jwrapper.c index a7c14e1..510899b 100644 --- a/src/json/jwrapper.c +++ b/src/json/jwrapper.c @@ -181,12 +181,10 @@ static void nix_things( void* st, void* se, const char* name, void* ele, void *d jthing_t* jarray; int i; - st = st; // silly things to keep sonar from complaining - name = name; - se = se; - data = data; - if( (j = (jthing_t *) ele) == NULL ) { + if( st == NULL && name == NULL && se == NULL && data == NULL ) { // these are ignored, but this keeps sonar from screaming bug + fprintf( stderr, "jwrapper: nix_thigs: all params were nil\n" ); + } return; } @@ -228,10 +226,14 @@ static void nix_things( void* st, void* se, const char* name, void* ele, void *d symtab code which defines the set of params and we use what we need. */ static void nix_mgt( void* st, void* se, const char* name, void* ele, void *data ) { - st = st; // silly things to keep sonar from complaining (let's hope the compiler is better than sonar - name = name; // and optimises these out). - se = se; - data = data; + + if( ele == NULL ) { + if( st == NULL && name == NULL && se == NULL && data == NULL ) { // these are ignored, but this keeps sonar from screaming bug + fprintf( stderr, "jwrapper: dump_things: all params were nil\n" ); + } + + return; + } free( ele ); } @@ -243,15 +245,13 @@ static void nix_mgt( void* st, void* se, const char* name, void* ele, void *dat static void dump_things( void* st, void* se, const char* name, void* ele, void *data ) { const jthing_t* j; - st = st; // silly things to keep sonar from complaining (let's hope the compiler is better than sonar - name = name; // and optimises these out). - se = se; - data = data; - j = (jthing_t *) ele; if( j ) { fprintf( stderr, " jwrapper: element '%s' has ptype %d, jsmn type %d\n", name, j->prim_type, j->jsmn_type ); } else { + if( st == NULL && name == NULL && se == NULL && data == NULL ) { // these are ignored, but this keeps sonar from screaming bug + fprintf( stderr, "jwrapper: dump_things: all params were nil\n" ); + } fprintf( stderr, " jwrapper: element has no data: '%s'\n", name ); } } diff --git a/src/xapp/xapp.cpp b/src/xapp/xapp.cpp index cb14f2e..caec394 100644 --- a/src/xapp/xapp.cpp +++ b/src/xapp/xapp.cpp @@ -87,7 +87,7 @@ void Xapp::Run( int nthreads ) { tinfo[i]->join(); } - delete tinfo; + delete[] tinfo; } /* diff --git a/test/unit_test.sh b/test/unit_test.sh index a4f9d0a..b49f81b 100755 --- a/test/unit_test.sh +++ b/test/unit_test.sh @@ -117,8 +117,11 @@ echo "tests successfully built" >&2 spew="cat" +# order here is important to ensure coverage files accumulate +tests="metrics_test jhash_test config_test unit_test" + #run everything, then generate coverage stats after all have run -for x in metrics_test jhash_test config_test unit_test +for x in $tests do ./$x >/tmp/PID$$.log 2>&1 abort_if_error $? "test failed: $x" @@ -127,7 +130,7 @@ done # it seems that we loose coverage reporting if metrics_test's gcov file is generated # after unit test. Very strange. To be safe, run unit_test last. # -for x in metrics_test jhash_test config_test unit_test +for x in $tests do gcov $x.c >/dev/null 2>&1 done -- 2.16.6