From: Alexandre Huff Date: Fri, 7 Jan 2022 17:20:28 +0000 (-0300) Subject: Re-enable RMR libary's module tests X-Git-Tag: 4.8.1^0 X-Git-Url: https://gerrit.o-ran-sc.org/r/gitweb?a=commitdiff_plain;h=fa454008020483ac35e1d20300cddfe877d8dd6d;p=ric-plt%2Flib%2Frmr.git Re-enable RMR libary's module tests This change fixes and re-enables the SI95's module tests disabled in RIC-838. This change also creates a new unit test for the debugging rmr rx queue API. Fixes some bugs and possible memory likeage in SI95 code. Issue-ID: RIC-861 Signed-off-by: Alexandre Huff Change-Id: I19e3bccb61605a8506b03afd755e627d0259c394 --- diff --git a/CHANGES_CORE.txt b/CHANGES_CORE.txt index 2aacd75..1843414 100644 --- a/CHANGES_CORE.txt +++ b/CHANGES_CORE.txt @@ -5,6 +5,11 @@ # API and build change and fix summaries. Doc corrections # and/or changes are not mentioned here; see the commit messages. +2022 January 7; version 4.8.1 + Re-enables RMR libary's module tests (RIC-861). + Creates a new unit test for the debugging rmr rx queue API. + Fixes some bugs and possible memory likeage in SI95 code. + 2021 December 2; version 4.8.0 Fixing memory leak in python support function (RIC-858). New API added for debugging rmr rx queue (RIC-838). diff --git a/CMakeLists.txt b/CMakeLists.txt index 03af953..93d2733 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -42,7 +42,7 @@ cmake_minimum_required( VERSION 3.5 ) set( major_version "4" ) # should be automatically populated from git tag later, but until CI process sets a tag we use this set( minor_version "8" ) -set( patch_level "0" ) +set( patch_level "1" ) set( install_root "${CMAKE_INSTALL_PREFIX}" ) set( install_inc "include/rmr" ) diff --git a/docs/rel-notes.rst b/docs/rel-notes.rst index 410bd10..8e85e60 100644 --- a/docs/rel-notes.rst +++ b/docs/rel-notes.rst @@ -22,6 +22,15 @@ the need to leap frog versions ceased, and beginning with version 4.0.0, the RMR versions should no longer skip. +2022 January 7; version 4.8.1 +------------------------------ + +Re-enables RMR libary's module tests (RIC-861). +Creates a new unit test for the debugging rmr rx queue API. +Fixes some bugs and possible memory likeage in SI95 code. + + + 2021 December 2; version 4.8.0 ------------------------------ diff --git a/src/rmr/si/src/rmr_debug_si.c b/src/rmr/si/src/rmr_debug_si.c index 3150139..2b4982c 100644 --- a/src/rmr/si/src/rmr_debug_si.c +++ b/src/rmr/si/src/rmr_debug_si.c @@ -70,7 +70,7 @@ extern int rmr_reset_rx_debug_count(void *vctx) { */ extern int rmr_get_rx_debug_info(void *vctx, rmr_rx_debug_t *rx_debug) { uta_ctx_t *ctx; - if ((ctx = (uta_ctx_t *)vctx) == NULL) { + if ((ctx = (uta_ctx_t *)vctx) == NULL || rx_debug == NULL ) { errno = EINVAL; return EINVAL; } diff --git a/src/rmr/si/src/si95/siaddress.c b/src/rmr/si/src/si95/siaddress.c index 153e6ac..1d76f71 100644 --- a/src/rmr/si/src/si95/siaddress.c +++ b/src/rmr/si/src/si95/siaddress.c @@ -44,7 +44,7 @@ * hostent structure. It claims that h_addr_list is a pointer * to character pointers, but it is really a pointer to a list * of pointers to integers!!! -* +* *************************************************************************** */ #include "sisetup.h" // get necessary defs and other stuff @@ -165,7 +165,7 @@ extern int SIaddress( void *src, void **dest, int type ) { (int) ntohs( addr6->sin6_port ) ); } else { num = (char *) &addr->sin_addr.s_addr; // point at the long - snprintf( wbuf, sizeof( wbuf ), "%u.%u.%u.%u;%d", *(num+0), *(num+1), *(num+2), *(num+3), (int) ntohs(addr->sin_port) ); + snprintf( wbuf, sizeof( wbuf ), "%u.%u.%u.%u:%d", *(num+0), *(num+1), *(num+2), *(num+3), (int) ntohs(addr->sin_port) ); } *dest = (void *) strdup( wbuf ); diff --git a/src/rmr/si/src/si95/siestablish.c b/src/rmr/si/src/si95/siestablish.c index 794ea07..2fa7e04 100644 --- a/src/rmr/si/src/si95/siestablish.c +++ b/src/rmr/si/src/si95/siestablish.c @@ -90,6 +90,7 @@ extern struct tp_blk *SIlisten_prep( int type, char* abuf, int family ) { if( addr != NULL ) { free( addr ); // not needed, but scanners complain if we don't overtly do this } + free( tptr ); return NULL; } @@ -189,6 +190,7 @@ extern struct tp_blk *SIconn_prep( struct ginfo_blk *gptr, int type, char *abuf, if( addr != NULL ) { // not needed, but scanners complain if we don't overtly do this free( addr ); } + free( tptr ); return NULL; } diff --git a/src/rmr/si/src/si95/siinit.c b/src/rmr/si/src/si95/siinit.c index b297124..0eaadb6 100644 --- a/src/rmr/si/src/si95/siinit.c +++ b/src/rmr/si/src/si95/siinit.c @@ -22,16 +22,16 @@ ************************************************************************** * Mnemonic: SIinitialise * Abstract: Initialisation and other context management functions. -* +* * Date: 26 March 1995 * Author: E. Scott Daniels * -* Mod: 17 FEB 2002 - To convert to a globally managed gpointer -* 09 Mar 2007 - To allow for ipv6 (added SIinitialise() to +* Mod: 17 FEB 2002 - To convert to a globally managed gpointer +* 09 Mar 2007 - To allow for ipv6 (added SIinitialise() to * replace SIinit()) ************************************************************************** */ -#include "sisetup.h" // get the setup stuff +#include "sisetup.h" // get the setup stuff /* Initialise the SI environment. Specifically: @@ -42,12 +42,12 @@ */ extern struct ginfo_blk* SIinitialise( int opts ) { - struct ginfo_blk *gptr = NULL; // pointer at gen info blk - int status = SI_OK; // status of internal processing - struct tp_blk *tpptr; // pointer at tp stuff - struct sigaction sact; // signal action block - int i; // loop index - int signals = SI_DEF_SIGS; // signals to be set in SIsetsig + struct ginfo_blk *gptr = NULL; // pointer at gen info blk + int status = SI_OK; // status of internal processing + struct tp_blk *tpptr; // pointer at tp stuff + struct sigaction sact; // signal action block + int i; // loop index + int signals = SI_DEF_SIGS; // signals to be set in SIsetsig errno = ENOMEM; @@ -65,24 +65,25 @@ extern struct ginfo_blk* SIinitialise( int opts ) gptr->cbtab = (struct callback_blk *) malloc( (sizeof( struct callback_blk ) * MAX_CBS ) ); if( gptr->cbtab != NULL ) { - for( i = 0; i < MAX_CBS; i++ ) { // initialize callback table - gptr->cbtab[i].cbdata = NULL; // no data and no functions + for( i = 0; i < MAX_CBS; i++ ) { // initialize callback table + gptr->cbtab[i].cbdata = NULL; // no data and no functions gptr->cbtab[i].cbrtn = NULL; } - } else { // if call back table allocation failed - error off - SIshutdown( gptr ); // clean up any open fds + } else { // if call back table allocation failed - error off + SIshutdown( gptr ); // clean up any open fds + free( gptr->tp_map ); free( gptr ); - gptr = NULL; // dont allow them to continue + gptr = NULL; // dont allow them to continue } - } // end if gen infor block allocated successfully + } // end if gen infor block allocated successfully + - memset( &sact, 0, sizeof( sact ) ); sact.sa_handler = SIG_IGN; sigaction( SIGPIPE, &sact, NULL ); // ignore pipe signals as for some bloody reason linux sets this off if write to closed socket - return gptr; // all's well that ends well -} + return gptr; // all's well that ends well +} /* This will set all of the tcp oriented flags in mask (SI_TF_* constants). diff --git a/src/rmr/si/src/si95/sinewses.c b/src/rmr/si/src/si95/sinewses.c index 56d9b10..8665892 100644 --- a/src/rmr/si/src/si95/sinewses.c +++ b/src/rmr/si/src/si95/sinewses.c @@ -52,6 +52,7 @@ extern int SInewsession( struct ginfo_blk *gptr, struct tp_blk *tpptr ) { addr = (struct sockaddr *) malloc( sizeof( struct sockaddr ) ); addrlen = sizeof( struct sockaddr ); + memset( addr, 0, sizeof( struct sockaddr ) ); status = accept( tpptr->fd, addr, &addrlen ); // accept and assign new fd (status) if( status < 0 ) { @@ -93,7 +94,7 @@ extern int SInewsession( struct ginfo_blk *gptr, struct tp_blk *tpptr ) { status = (*cbptr)( gptr->cbtab[SI_CB_SECURITY].cbdata, buf ); if( status == SI_RET_ERROR ) { // session to be rejected SIterm( gptr, newtp ); // terminate new tp block (do NOT call trash) - free( addr ); + // free( addr ); // not required, will be eventually freed by SItrash free( buf ); return SI_ERROR; } else { diff --git a/src/rmr/si/src/si95/sitrash.c b/src/rmr/si/src/si95/sitrash.c index 83671c7..33c6801 100644 --- a/src/rmr/si/src/si95/sitrash.c +++ b/src/rmr/si/src/si95/sitrash.c @@ -20,20 +20,20 @@ /* *************************************************************************** -* +* * Mnemonic: sitrash * Abstract: Delete all things referenced by a struct and then free the memory. -* +* * Returns: Nothing * Date: 08 March 2007 -* Author: E. Scott Daniels +* Author: E. Scott Daniels * -****************************************************************************** +****************************************************************************** */ #include "sisetup.h" #include "sitransport.h" - + extern void SItrash( int type, void *bp ) { struct tp_blk *tp = NULL; @@ -52,14 +52,14 @@ extern void SItrash( int type, void *bp ) free( iptr->addr ); free( iptr ); break; - - case TP_BLK: // we assume its off the list + + case TP_BLK: // we assume its off the list tp = (struct tp_blk *) bp; - iptr = tp->squeue; + iptr = tp->squeue; while( iptr != NULL ) { inext = iptr->next; - free( iptr->data ); // we could recurse, but that seems silly + free( iptr->data ); // we could recurse, but that seems silly free( iptr->addr ); free( iptr ); @@ -69,10 +69,10 @@ extern void SItrash( int type, void *bp ) if( tp->fd >= 0 ) { CLOSE( tp->fd ); } - - free( tp->addr ); // release the address bufers - free( tp->paddr ); - free( tp ); // and release the block + + free( tp->addr ); // release the address bufers + free( tp->paddr ); + free( tp ); // and release the block break; } } diff --git a/src/rmr/si/src/si95/siwait.c b/src/rmr/si/src/si95/siwait.c index 3a9a8ed..683bc5e 100644 --- a/src/rmr/si/src/si95/siwait.c +++ b/src/rmr/si/src/si95/siwait.c @@ -70,8 +70,6 @@ extern int SIwait( struct ginfo_blk *gptr ) { struct tp_blk *nextone= NULL; // point at next block to process in loop int pstat = 0; // poll status struct timeval timeout; // delay to use on select call - char *buf = NULL; - char *ibuf = NULL; if( gptr->magicnum != MAGICNUM ) { // if not a valid ginfo block rmr_vlog( RMR_VL_CRIT, "SI95: wait: bad global info struct magic number is wrong\n" ); @@ -82,11 +80,6 @@ extern int SIwait( struct ginfo_blk *gptr ) { return SI_ERROR; // so just get out } - if( ( ibuf = (char *) malloc( 2048 ) ) == NULL ) { - rmr_vlog( RMR_VL_WARN, "ibuf malloc fail\n" ); - return SI_ERROR; - } - do { // spin until a callback says to stop (likely never) timeout.tv_sec = 0; // must be reset on every call! timeout.tv_usec = SI_SELECT_TIMEOUT; @@ -147,7 +140,6 @@ extern int SIwait( struct ginfo_blk *gptr ) { } } while( gptr->tplist != NULL && !(gptr->flags & GIF_SHUTDOWN) ); - free( ibuf ); if( gptr->tplist == NULL ) // indicate all fds closed if( gptr->flags & GIF_SHUTDOWN ) { // we need to stop for some reason diff --git a/test/rmr_debug_si_test.c b/test/rmr_debug_si_test.c new file mode 100644 index 0000000..1a74a9d --- /dev/null +++ b/test/rmr_debug_si_test.c @@ -0,0 +1,127 @@ +// vim: ts=4 sw=4 noet +/* +================================================================================== + Copyright (c) 2021 Alexandre Huff (UTFPR). + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +================================================================================== +*/ + +/* + Mnemonic: rmr_debug_si_test.c + Abstract: This is the main driver to test the si95 debug functions + + Date: 24 December 2021 + Author: Alexandre Huff +*/ +#include +#include + +#define DEBUG 1 +#undef NNG_UNDER_TEST // NNG is NOT under test so undefine if set +#define NO_EMULATION 1 // no emulation of transport functions + +#include "rmr.h" +#include "rmr_agnostic.h" + +#include "si95/socket_if.h" +#undef uta_ctx_t +#include "rmr_si_private.h" + +#include "test_support.c" // things like fail_if() + +#include "rmr_debug_si.c" // api under test + + + + +// ------------- dummy functions to force edge cases when we can --------------------------------------- + +#define SYSTEM_UNDER_TEST 1 // for conditional code + + +// ---------------------------------------------------------------------------------------- + +static int get_debug_info_test( uta_ctx_t *ctx ) { + int errors = 0; + int ret; + rmr_rx_debug_t info; + + ctx->acc_dcount = 5; // dummy info + ctx->acc_ecount = 10; + + // argument related things + ret = rmr_get_rx_debug_info( NULL, &info ); + errors += fail_not_equal( EINVAL, errno, "get_debug_info_test: rmr_get_rx_debug_info did not set errno to EINVAL on nil global context" ); + errors += fail_if_equal( 0, ret, "get_debug_info_test: rmr_get_rx_debug_info returned 0 on error" ); + + errno = 0; + ret = rmr_get_rx_debug_info( ctx, NULL ); + errors += fail_not_equal( EINVAL, errno, "get_debug_info_test: rmr_get_rx_debug_info did not set errno to EINVAL on nil info struct" ); + errors += fail_if_equal( 0, ret, "get_debug_info_test: rmr_get_rx_debug_info returned 0 on error" ); + + // test rx debug struct values + ret = rmr_get_rx_debug_info( ctx, &info ); + errors += fail_not_equal( 0, ret, "get_debug_info_test: rmr_get_rx_debug_info did not return 0 on success" ); + errors += fail_not_equal( 5, ctx->acc_dcount, "get_debug_info_test: rmr_get_rx_debug_info unexpected acc_dcount value in info struct" ); + errors += fail_not_equal( 10, ctx->acc_ecount, "get_debug_info_test: rmr_get_rx_debug_info unexpected acc_ecount value in info struct" ); + + fprintf( stderr, " get_debug_info_test finished with %d errors\n", errors ); + + return errors; +} + +static int reset_debug_test(uta_ctx_t *ctx) { + int errors = 0; + int ret; + + ctx->acc_dcount = 5; // dummy info + ctx->acc_ecount = 10; + + ret = rmr_reset_rx_debug_count( NULL ); // expect to fail + errors += fail_not_equal( EINVAL, errno, "reset_debug_test: rmr_reset_rx_debug_count did not set errno to EINVAL on error" ); + errors += fail_if_equal( 0, ret, "reset_debug_test: rmr_reset_rx_debug_count returned 0 on error" ); + + ret = rmr_reset_rx_debug_count( ctx ); // expect to return successfully + errors += fail_not_equal( 0, ret, "reset_debug_test: reset_debug_rx_count did not return 0 on success" ); + errors += fail_not_equal( 0, ctx->acc_dcount, "reset_debug_test: rmr_reset_rx_debug_count did not reset acc_dcount to 0" ); + errors += fail_not_equal( 0, ctx->acc_ecount, "reset_debug_test: rmr_reset_rx_debug_count did not reset acc_ecount to 0" ); + + fprintf( stderr, " reset_debug_test finished with %d errors\n", errors ); + + return errors; +} + +// ---------------------------------------------------------------------------------------- + +/* + Drive tests... +*/ +int main() { + uta_ctx_t si_ctx; + int errors = 0; + + fprintf( stderr, "\n starting SI95 debug api tests\n" ); + + errors += get_debug_info_test( &si_ctx ); + errors += reset_debug_test( &si_ctx ); + + test_summary( errors, "SI95 debug api tests" ); + if( errors == 0 ) { + fprintf( stderr, " all tests were OK\n\n" ); + } else { + fprintf( stderr, " %d errors in SI95 debug api code\n\n", errors ); + } + + return !!errors; +} diff --git a/test/si95_test_fixme.c b/test/si95_test.c similarity index 96% rename from test/si95_test_fixme.c rename to test/si95_test.c index 8e9ee50..fb6e7fd 100644 --- a/test/si95_test_fixme.c +++ b/test/si95_test.c @@ -32,7 +32,6 @@ #include #include #include -#include #include #include #include @@ -74,7 +73,7 @@ static int bad_mallocs = 1; // number of failed mallocs (consecutive) static void* test_malloc( size_t n ) { -fprintf( stderr, ">>>> test malloc: %d %d\n", good_mallocs, bad_mallocs ); + fprintf( stderr, ">>>> test malloc: %d %d\n", good_mallocs, bad_mallocs ); if( good_mallocs ) { good_mallocs--; return malloc( n ); @@ -164,6 +163,7 @@ static int memory( ) { ptr = SInew( GI_BLK ); errors += fail_if_nil( ptr, "memory: sinew returned nil when given giblk request" ); SItrash( GI_BLK, ptr ); // GI block cannot be trashed, ensure this (valgind will complain about a leak) + free( ptr ); // we can free GI block only in tests fprintf( stderr, " memory module finished with %d errors\n", errors ); return errors; @@ -204,6 +204,13 @@ static int cleanup() { SIshutdown( NULL ); SIabort( si_ctx ); + // cleaning up the remaining global resources + struct ginfo_blk *gptr = (struct ginfo_blk*)si_ctx; + SItrash( TP_BLK, gptr->tplist ); + free( gptr->tp_map ); + free( gptr->rbuf ); + free( gptr->cbtab ); + free( si_ctx ); fprintf( stderr, " cleanup module finished with %d errors\n", errors ); return errors; @@ -215,13 +222,14 @@ static int cleanup() { static int addr() { int errors = 0; int l; - struct sockaddr* addr; char buf1[4096]; // space to build buffers for xlation char* hr_addr; // human readable address returned void* net_addr; // a network address block of some type - addr = (struct sockaddr *) malloc( sizeof( struct sockaddr ) ); /* + struct sockaddr* addr; + addr = (struct sockaddr *) malloc( sizeof( struct sockaddr ) ); + l = SIgenaddr( " [ff02::4]:4567", PF_INET6, IPPROTO_TCP, SOCK_STREAM, &addr ); SIgenaddr( " [ff02::4]:4567", PF_INET6, IPPROTO_TCP, SOCK_STREAM, &addr ); @@ -247,6 +255,7 @@ static int addr() { errors += fail_if_true( l < 1, "v6 to dot conversion failed" ); errors += fail_if_nil( hr_addr, "v6 to dot conversion yields a nil pointer" ); free( net_addr ); + free( hr_addr ); } snprintf( buf1, sizeof( buf1 ), "localhost:43086" ); @@ -257,6 +266,7 @@ static int addr() { errors += fail_if_true( l < 1, "to dot convdersion failed" ); errors += fail_if_nil( hr_addr, "v4 to dot conversion yields a nil pointer" ); free( net_addr ); + free( hr_addr ); fprintf( stderr, " addr module finished with %d errors\n", errors ); return errors; @@ -272,6 +282,7 @@ static int prep() { thing = SIlisten_prep( UDP_DEVICE, "localhost:1234", AF_INET ); errors += fail_if_nil( thing, "listen prep udp returned nil block" ); + SItrash( TP_BLK, thing ); thing = SIlisten_prep( UDP_DEVICE, "localhost:1234", 84306 ); // this should fail errors += fail_not_nil( thing, "listen prep udp returned valid block ptr for bogus family" ); @@ -296,9 +307,15 @@ static int poll() { dummy->flags |= GIF_SHUTDOWN; // shutdown edge condition SIpoll( dummy, 1 ); + free( dummy->tp_map ); + free( dummy->rbuf ); + free( dummy->cbtab ); + memset( dummy, 0, sizeof( *dummy ) ); // force bad cookie check code to drive SIpoll( dummy, 1 ); + free (dummy ); + status = SIpoll( si_ctx, 1 ); errors += fail_if_true( status != 0, "poll failed" ); @@ -426,6 +443,8 @@ static int new_sess( ) { status = SInewsession( si_ctx, tpptr ); errors += fail_if_true( status < 0, "newsession did failed when accept was good" ); + free( tpptr ); + fprintf( stderr, " new_sess module finished with %d errors\n", errors ); return errors; } @@ -479,9 +498,16 @@ static int wait_tests() { dummy->flags |= GIF_SHUTDOWN; SIwait( dummy ); + free( dummy->tp_map ); + free( dummy->rbuf ); + free( dummy->cbtab ); + memset( dummy, 0, sizeof( *dummy ) ); // force bad cookie check code to drive SIwait( dummy ); + free( dummy ); + + SIwait( si_ctx ); // should drive once through the loop return errors;