From 05850e0815095c029ecff43ac8e0983f2fba4fb6 Mon Sep 17 00:00:00 2001 From: "E. Scott Daniels" Date: Wed, 11 Nov 2020 15:57:22 -0500 Subject: [PATCH] More changes for scan corrections and unit test coverage Issue-ID: RIC-673 Signed-off-by: E. Scott Daniels Change-Id: I337e8caebda4d4585fbce2ab455caaa4f68be8f1 --- CHANGES_CORE.txt | 4 ++ CMakeLists.txt | 2 +- src/rmr/common/src/rt_generic_static.c | 5 ++- src/rmr/common/src/rtc_static.c | 12 ++--- src/rmr/si/src/rmr_si.c | 2 + src/rmr/si/src/si95/siaddress.c | 2 +- src/rmr/si/src/si95/siestablish.c | 11 +++-- src/rmr/si/src/si95/sigetname.c | 2 +- src/rmr/si/src/si95/silisten.c | 2 +- src/rmr/si/src/si95/sipoll.c | 2 +- src/rmr/si/src/si95/siproto.h | 2 +- test/rt_static_test.c | 59 +++++++++++++++++++++---- test/si95_test.c | 80 +++++++++++++++++++++++++++++----- test/test_gen_rt.c | 17 +++++--- test/test_si95_em.c | 2 +- 15 files changed, 159 insertions(+), 45 deletions(-) diff --git a/CHANGES_CORE.txt b/CHANGES_CORE.txt index fa92133..d448a57 100644 --- a/CHANGES_CORE.txt +++ b/CHANGES_CORE.txt @@ -5,6 +5,10 @@ # API and build change and fix summaries. Doc correctsions # and/or changes are not mentioned here; see the commit messages. +2020 November 4; Version 4.4.2 + Changes to correct more complaints generated by a code scan. (RIC-673) + Also addressed some sonar coverage issues with unit test changes. + 2020 November 4; Version 4.4.1 Changes to correct complaints generated by a code scan. (RIC-673) diff --git a/CMakeLists.txt b/CMakeLists.txt index dd189f4..8705898 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -41,7 +41,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 "4" ) -set( patch_level "1" ) +set( patch_level "2" ) set( install_root "${CMAKE_INSTALL_PREFIX}" ) set( install_inc "include/rmr" ) diff --git a/src/rmr/common/src/rt_generic_static.c b/src/rmr/common/src/rt_generic_static.c index 532a6bf..84f8445 100644 --- a/src/rmr/common/src/rt_generic_static.c +++ b/src/rmr/common/src/rt_generic_static.c @@ -389,6 +389,7 @@ static char* ensure_nlterm( char* buf ) { is no active table (first load), so we have to account for that (no locking). */ static void roll_tables( uta_ctx_t* ctx ) { + if( ctx->rtable != NULL ) { // initially there isn't one, so must check! pthread_mutex_lock( ctx->rtgate ); // must hold lock to move to active ctx->old_rtable = ctx->rtable; // currently active becomes old and allowed to 'drain' @@ -1304,8 +1305,8 @@ static route_table_t* prep_new_rt( uta_ctx_t* ctx, int all ) { rt = NULL; } - rt = uta_rt_clone( ctx, ctx->rtable, rt, all ); // also sets the ephash pointer - rt->ref_count = 0; // take no chances; ensure it's 0! + rt = uta_rt_clone( ctx, ctx->rtable, rt, all ); // also sets the ephash pointer + rt->ref_count = 0; // take no chances; ensure it's 0! return rt; } diff --git a/src/rmr/common/src/rtc_static.c b/src/rmr/common/src/rtc_static.c index 9aca092..15fe567 100644 --- a/src/rmr/common/src/rtc_static.c +++ b/src/rmr/common/src/rtc_static.c @@ -76,9 +76,8 @@ static void* rtc_file( void* vctx ) { ctx->flags |= CFL_NO_RTACK; // no attempt to ack when reading from a file while( 1 ) { if( vfd >= 0 ) { - wbuf[0] = 0; - lseek( vfd, 0, 0 ); - if( read( vfd, wbuf, 10 ) > 0 ) { + memset( wbuf, 0, sizeof( char ) * 11 ); + if( lseek( vfd, 0, SEEK_SET ) == 0 && read( vfd, wbuf, 10 ) > 0 ) { vlevel = atoi( wbuf ); } } @@ -86,6 +85,9 @@ static void* rtc_file( void* vctx ) { read_static_rt( ctx, vlevel ); // seed the route table if one provided if( ctx->shutdown != 0 ) { // allow for graceful termination and unit testing + if( vfd >= 0 ) { + close( vfd ); + } return NULL; } sleep( 60 ); @@ -98,8 +100,8 @@ static int refresh_vlevel( int vfd ) { if( vfd >= 0 ) { // if file is open, read current value rbuf[0] = 0; - lseek( vfd, 0, 0 ); - if( read( vfd, rbuf, 10 ) > 0 ) { + memset( rbuf, 0, sizeof( char ) * 11 ); + if( lseek( vfd, 0, SEEK_SET ) == 0 && read( vfd, rbuf, 10 ) > 0 ) { vlevel = atoi( rbuf ); } } diff --git a/src/rmr/si/src/rmr_si.c b/src/rmr/si/src/rmr_si.c index 85e058b..bbc47a5 100644 --- a/src/rmr/si/src/rmr_si.c +++ b/src/rmr/si/src/rmr_si.c @@ -696,6 +696,7 @@ static void* init( char* uproto_port, int def_msg_size, int flags ) { snprintf( bind_info, sizeof( bind_info ), "%s:%s", interface, port ); // FIXME -- si only supports 0.0.0.0 by default if( (state = SIlistener( ctx->si_ctx, TCP_DEVICE, bind_info )) < 0 ) { rmr_vlog( RMR_VL_CRIT, "rmr_init: unable to start si listener for %s: %s\n", bind_info, strerror( errno ) ); + free( proto_port ); // some scanners might complain that port is not freed; it CANNOT be free_ctx( ctx ); return NULL; } @@ -713,6 +714,7 @@ static void* init( char* uproto_port, int def_msg_size, int flags ) { ctx->ephash = rmr_sym_alloc( 129 ); // host:port to ep symtab exists outside of any route table if( ctx->ephash == NULL ) { rmr_vlog( RMR_VL_CRIT, "rmr_init: unable to allocate ep hash\n" ); + free( proto_port ); // some scanners might complain that port is not freed; it CANNOT be free_ctx( ctx ); return NULL; } diff --git a/src/rmr/si/src/si95/siaddress.c b/src/rmr/si/src/si95/siaddress.c index 6533f33..153e6ac 100644 --- a/src/rmr/si/src/si95/siaddress.c +++ b/src/rmr/si/src/si95/siaddress.c @@ -159,7 +159,7 @@ extern int SIaddress( void *src, void **dest, int type ) { if( addr->sin_family == AF_INET6 ) { addr6 = (struct sockaddr_in6 *) src; // really an ip6 struct byte = (uint8_t *) &addr6->sin6_addr; - snprintf( wbuf, sizeof( wbuf ), "[%u:%u:%u:%u:%u:%u]:%d", + snprintf( wbuf, sizeof( wbuf ), "[%u:%u:%u:%u:%u:%u]:%d", *(byte+0), *(byte+1), *(byte+2), *(byte+3), *(byte+4), *(byte+5) , (int) ntohs( addr6->sin6_port ) ); diff --git a/src/rmr/si/src/si95/siestablish.c b/src/rmr/si/src/si95/siestablish.c index d3068e0..a62490d 100644 --- a/src/rmr/si/src/si95/siestablish.c +++ b/src/rmr/si/src/si95/siestablish.c @@ -57,19 +57,18 @@ Family is one of the AF_* constants (AF_ANY, AF_INET or AF_INET6) The address should be one of these forms: - [::1]:port // v6 localhost device (loop back) - localhost:port // v4 or 6 loopback depending on /etc/hosts - 0.0.0.0:port // any interface - addr:port // an address assigned to one of the devices + [::1]:port v6 localhost device (loop back) + localhost:port v4 or 6 loopback depending on /etc/hosts + 0.0.0.0:port any interface + addr:port an address assigned to one of the devices Returns a transport struct which is the main context for the listener. */ -extern struct tp_blk *SIlisten_prep( struct ginfo_blk *gptr, int type, char* abuf, int family ) { +extern struct tp_blk *SIlisten_prep( int type, char* abuf, int family ) { struct tp_blk *tptr; // pointer at new tp block int status = SI_OK; // processing status struct sockaddr *addr; // IP address we are requesting int protocol; // protocol for socket call - char buf[256]; // buffer to build request address in int optval = 0; int alen = 0; diff --git a/src/rmr/si/src/si95/sigetname.c b/src/rmr/si/src/si95/sigetname.c index fee7199..9fc1a77 100644 --- a/src/rmr/si/src/si95/sigetname.c +++ b/src/rmr/si/src/si95/sigetname.c @@ -38,7 +38,7 @@ extern char* SIgetname( int sid ) { int len; len = sizeof( oaddr ); - if( getsockname( sid, &oaddr, &len ) < 0 ) { + if( sid < 0 || getsockname( sid, &oaddr, &len ) < 0 || len != sizeof( oaddr ) ) { return NULL; } diff --git a/src/rmr/si/src/si95/silisten.c b/src/rmr/si/src/si95/silisten.c index 3ce0e18..38ba600 100644 --- a/src/rmr/si/src/si95/silisten.c +++ b/src/rmr/si/src/si95/silisten.c @@ -53,7 +53,7 @@ extern int SIlistener( struct ginfo_blk *gptr, int type, char *abuf ) { return status; } - tpptr = SIlisten_prep( gptr, type, abuf, 0 ); + tpptr = SIlisten_prep( type, abuf, 0 ); if( tpptr != NULL ) // established a fd bound to the port ok { // enable connection reqs diff --git a/src/rmr/si/src/si95/sipoll.c b/src/rmr/si/src/si95/sipoll.c index aa85e8b..6c8915a 100644 --- a/src/rmr/si/src/si95/sipoll.c +++ b/src/rmr/si/src/si95/sipoll.c @@ -74,7 +74,7 @@ extern int SIpoll( struct ginfo_blk *gptr, int msdelay ) pstat = select( gptr->fdcount, &gptr->readfds, &gptr->writefds, &gptr->execpfds, &delay ); - if( (pstat < 0 && errno != EINTR) ) + if( pstat < 0 && errno != EINTR ) { // poll fail or termination signal rcvd gptr->fdcount = 0; // prevent trying to look at a session gptr->flags |= GIF_SHUTDOWN; // cause cleanup and exit at end diff --git a/src/rmr/si/src/si95/siproto.h b/src/rmr/si/src/si95/siproto.h index 93bdd7b..7f9fee6 100644 --- a/src/rmr/si/src/si95/siproto.h +++ b/src/rmr/si/src/si95/siproto.h @@ -47,7 +47,7 @@ extern int SIconnect( struct ginfo_blk *gptr, char *abuf ); extern struct tp_blk *SIestablish( int type, char *abuf, int family ); extern int SIgenaddr( char *target, int proto, int family, int socktype, struct sockaddr **rap ); extern int SIgetaddr( struct ginfo_blk *gptr, char *buf ); -extern struct tp_blk *SIlisten_prep( struct ginfo_blk *gptr, int type, char* abuf, int family ); +extern struct tp_blk *SIlisten_prep( int type, char* abuf, int family ); extern int SIlistener( struct ginfo_blk *gptr, int type, char *abuf ); extern void SImap_fd( struct ginfo_blk *gptr, int fd, struct tp_blk* tpptr ); extern int SInewsession( struct ginfo_blk *gptr, struct tp_blk *tpptr ); diff --git a/test/rt_static_test.c b/test/rt_static_test.c index 8fd91ac..8bd91df 100644 --- a/test/rt_static_test.c +++ b/test/rt_static_test.c @@ -415,19 +415,57 @@ static int rt_test( ) { free( buf ); } -fprintf( stderr, ">>>>>> test is overtly dropping rt table at %p\n", rt ); + fprintf( stderr, " test is overtly dropping rt table at %p\n", rt ); + ctx->rtable = NULL; uta_rt_drop( rt ); rt = NULL; + // --- force the load of a RT which has some edge case forcing issues if( ctx ) { - if( (seed_fname = getenv( "RMR_SEED_RT" )) != NULL ) { - read_static_rt( ctx, 0 ); - rt = ctx->rtable; - errors += fail_if_nil( rt, "read seed table didn't generate a rtable pointer in context" ); - unsetenv( "RMR_SEED_RT" ); // remove for next test - } - + char* rt_stuff = + "newrt | start | dummy-seed\n" + "mse | 1 | -1 | localhost:84306\n" + "mse | 10 | -1 | localhost:84306\n" + "mse | 10 | 1 | localhost:84306\n" + "# should cause failure because there aren't 10 entries above\n" + "newrt | end | 10\n" + + "# this table has no end\n" + "newrt | start | dummy-seed\n" + "mse | 1 | -1 | localhost:84306\n" + "mse | 10 | -1 | localhost:84306\n" + "mse | 10 | 1 | localhost:84306\n" + "# short record to drive test\n" + "del\n" + "del | 12 | 12\n" + + "# this table should be ok\n" + "newrt | start | dummy-seed\n" + "mse | 1 | -1 | localhost:84306\n" + "mse | 10 | -1 | localhost:84306\n" + "mse | 10 | 1 | localhost:84306\n" + "newrt | end | 3\n" + + "# for an update to the existing table\n" + + "# not in progress; drive that exception check\n" + "update | end | 23\n" + + "update | start | dummy-seed\n" + "mse | 2 | 2 | localhost:2222\n" + "# no table end for exception handling\n" + + "update | start | dummy-seed\n" + "mse | 2 | 2 | localhost:2222\n" + "update | end | 1\n"; + + fprintf( stderr, " loading RT from edge case static table\n" ); + fprintf( stderr, " %s\n", rt_stuff ); + gen_custom_rt( ctx, rt_stuff ); + errors += fail_if_nil( ctx->rtable, "edge case route table didn't generate a pointer into the context" ); + + unsetenv( "RMR_SEED_RT" ); // remove for next read try read_static_rt( ctx, 0 ); // drive for not there coverage } @@ -620,5 +658,10 @@ fprintf( stderr, ">>>>>> test is overtly dropping rt table at %p\n", rt ); errors += fail_not_nil( ep, "fd2ep delete returned a pointer for unknown mapping" ); #endif + // ---------------- misc coverage tests -------------------------------------------------------------------------- + collect_things( NULL, NULL, NULL, NULL, NULL ); // these both return null, these test NP checks + collect_things( NULL, NULL, NULL, NULL, (void *) 1234 ); // the last is an invalid pointer, but check needed to force check on previous param + del_rte( NULL, NULL, NULL, NULL, NULL ); + return !!errors; // 1 or 0 regardless of count } diff --git a/test/si95_test.c b/test/si95_test.c index dfa44c3..70f13e8 100644 --- a/test/si95_test.c +++ b/test/si95_test.c @@ -88,9 +88,9 @@ #include #include #include -//#include +#include //#include -//#include +#include #include #include #include @@ -197,9 +197,9 @@ static int addr() { int errors = 0; int l; struct sockaddr* addr; - char buf1[4096]; - char buf2[4096]; - char* dest; + 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 ) ); /* @@ -208,26 +208,75 @@ static int addr() { SIgenaddr( " [ff02::4]:4567", PF_INET6, IPPROTO_TCP, SOCK_STREAM, &addr ); */ - dest = NULL; + l = SIaddress( NULL, NULL, 0 ); + errors += fail_if_true( l != 0, "SIaddress given two null pointers didn't return 0 len" ); + l = SIaddress( buf1, NULL, 0 ); + errors += fail_if_true( l != 0, "SIaddress given null dest pointer didn't return 0 len" ); + l = SIaddress( NULL, buf1, 0 ); + errors += fail_if_true( l != 0, "SIaddress given null src pointer didn't return 0 len" ); + + net_addr = NULL; snprintf( buf1, sizeof( buf1 ), " [ff02::5:4001" ); // invalid address, drive leading space eater too - l = SIaddress( buf1, (void **) &dest, AC_TOADDR6 ); + l = SIaddress( buf1, (void **) &net_addr, AC_TOADDR6 ); errors += fail_if_true( l > 0, "to addr6 with bad addr convdersion returned valid len" ); + free( net_addr ); snprintf( buf1, sizeof( buf1 ), "[ff02::5]:4002" ); // v6 might not be supported so failure is OK here; driving for coverage - l=SIaddress( buf1, (void **) &dest, AC_TOADDR6 ); + l = SIaddress( buf1, &net_addr, AC_TOADDR6 ); + if( l > 0 ) { + l = SIaddress( net_addr, &hr_addr, AC_TODOT ); // convert the address back to hr string + 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 ); + } snprintf( buf1, sizeof( buf1 ), "localhost:43086" ); - l = SIaddress( buf1, (void **) &dest, AC_TOADDR ); - errors += fail_if_true( l < 1, "to addr convdersion failed" ); + l = SIaddress( buf1, (void **) &net_addr, AC_TOADDR ); + errors += fail_if_true( l < 1, "v4 to addr conversion failed" ); - snprintf( buf1, sizeof( buf1 ), "localhost:4004" ); - l = SIaddress( buf1, (void **) &dest, AC_TODOT ); + l = SIaddress( net_addr, &hr_addr, AC_TODOT ); // convert the address back to hr string 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 ); fprintf( stderr, " addr module finished with %d errors\n", errors ); return errors; } +/* + Prep related tests. These mostly drive cases that aren't driven by "normal" + connect, send, receive tests (e.g. UDP branches). +*/ +static int prep() { + int errors = 0; + void* thing; // the thing that should be returned + + thing = SIlisten_prep( UDP_DEVICE, "localhost:1234", AF_INET ); + errors += fail_if_nil( thing, "listen prep udp returned nil block" ); + + 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" ); + + thing = SIconn_prep( si_ctx, UDP_DEVICE, "localhost:1234", 84306 ); // again, expect to fail; bogus family + errors += fail_not_nil( thing, "conn prep udp returned valid block ptr for bogus family" ); + + return errors; +} + +/* + Polling/waiting tests. These are difficult at best because of the blocking + nature of things, not to mention needing to have real ports open etc. +*/ +static int poll() { + int errors = 0; + int status; + + status = SIpoll( si_ctx, 1 ); + errors += fail_if_true( status != 0, "poll failed" ); + + return errors; +} + /* Connection oriented tests. @@ -290,8 +339,12 @@ static int conn( ) { errors++; } else { errors += fail_if_true( buf[0] == 0, "get name returned buf with emtpy string" ); + free( buf ); } + buf = SIgetname( -1 ); // invalid fd + errors += fail_not_nil( buf, "get name returned buf with non-emtpy string when given bad fd" ); + fprintf( stderr, " conn module finished with %d errors\n", errors ); return errors; } @@ -398,12 +451,15 @@ int main() { errors += init(); errors += memory(); errors += addr(); + errors += prep(); errors += conn(); errors += misc(); errors += new_sess(); // should leave a "connected" session at fd == 6 errors += send_tests(); + errors += poll(); + errors += cleanup(); test_summary( errors, "SI95 tests" ); diff --git a/test/test_gen_rt.c b/test/test_gen_rt.c index 08c1276..5d9e366 100644 --- a/test/test_gen_rt.c +++ b/test/test_gen_rt.c @@ -37,6 +37,9 @@ is a good update. The same applies to the meid map; first has a bad counter and some bad records to drive coverage testing. The end should leave a good meid map in the table. + + Once the file is generated, it is forced in via static read and the file + is unlinked. */ static void gen_rt( uta_ctx_t* ctx ) { int fd; @@ -72,7 +75,7 @@ static void gen_rt( uta_ctx_t* ctx ) { rt_stuff = // add an meid map which will fail "meid_map | start\n" - "mme_ar | e2t-1 | one two three four\n" + "mme_ar | e2t-1 | one two three four\r" // also test bloody apple way with \r "mme_del | one two\n" "mme_del \n" // short entries drive various checks for coverage "mme_ar \n" @@ -118,19 +121,23 @@ static void gen_rt( uta_ctx_t* ctx ) { /* Generate a custom route table file using the buffer passed in. + Then force it to be read into the current route table. The file + is unlinked when finished. */ static void gen_custom_rt( uta_ctx_t* ctx, char* buf ) { int fd; - char* rt_stuff; // strings for the route table - fd = open( "utesting.rt", O_WRONLY | O_CREAT, 0600 ); + fd = open( "Xutesting.rt", O_WRONLY | O_CREAT, 0600 ); if( fd < 0 ) { fprintf( stderr, " unable to open file for testing route table gen\n" ); return; } - setenv( "RMR_SEED_RT", "utesting.rt", 1 ); + setenv( "RMR_SEED_RT", "Xutesting.rt", 1 ); - write( fd, rt_stuff, strlen( buf ) ); + fprintf( stderr, " creating custom rt from buffer (%d bytes)\n", strlen (buf ) ); + if( write( fd, buf, strlen( buf ) ) != strlen( buf ) ) { + fprintf( stderr, " write failed: %s\n", strerror( errno ) ); + } close( fd ); read_static_rt( ctx, 1 ); // force in verbose mode to see stats on tty if failure diff --git a/test/test_si95_em.c b/test/test_si95_em.c index 9eb7d94..3e70727 100644 --- a/test/test_si95_em.c +++ b/test/test_si95_em.c @@ -173,7 +173,7 @@ static int em_sigetaddr( struct ginfo_blk *gptr, char *buf ) { return 0; } -static struct tp_blk *em_silisten_prep( struct ginfo_blk *gptr, int type, char* abuf, int family ) { +static struct tp_blk *em_silisten_prep( int type, char* abuf, int family ) { return NULL; } -- 2.16.6