From: E. Scott Daniels Date: Fri, 30 Oct 2020 19:04:16 +0000 (-0400) Subject: Address code analysis issues X-Git-Tag: 4.3.1^0 X-Git-Url: https://gerrit.o-ran-sc.org/r/gitweb?a=commitdiff_plain;h=fcea3951d44de0cc55d33c5e114487abe79d3406;p=ric-plt%2Flib%2Frmr.git Address code analysis issues This change addresses code analysis issues which were found to be potential bugs in the code. The details are captured in the Jira ticket. Issue-ID: RIC-673 Signed-off-by: E. Scott Daniels Change-Id: Ic05b716780f5fae76e98a3c5fa8be98fcd8452d8 --- diff --git a/CHANGES_CORE.txt b/CHANGES_CORE.txt index e8a418b..e991000 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 October 30; Version 4.3.1 + Changes to address code analyser scans and two bug fixes identified + while addressing the analysis data. (RIC-673) + 2020 October 2; Version 4.3.0 Add message types for traffic steering anomaly messages diff --git a/CMakeLists.txt b/CMakeLists.txt index 66e0d74..bd58d84 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 "3" ) -set( patch_level "0" ) +set( patch_level "1" ) set( install_root "${CMAKE_INSTALL_PREFIX}" ) set( install_inc "include/rmr" ) diff --git a/doc/src/rst.im b/doc/src/rst.im index 3510c24..2702fd7 100644 --- a/doc/src/rst.im +++ b/doc/src/rst.im @@ -66,7 +66,7 @@ .dv h2 .h2 $1 .dv h3 .h2 $1 .ei - .dv __alert ### WARNING ### rst.im detects an old(er) version of tfm some formatting might not be right + .dv __alert ### WARNING ### rst.im detects an old(er) version of tfm some formatting might not be right (&_major) .sv __alert .dh 1 s=2,1 i=0 m=0 diff --git a/docs/rel-notes.rst b/docs/rel-notes.rst index 948016c..d244c0c 100644 --- a/docs/rel-notes.rst +++ b/docs/rel-notes.rst @@ -22,10 +22,18 @@ the need to leap frog versions ceased, and beginning with version 4.0.0, the RMR versions should no longer skip. +2020 October 30; Version 4.3.1 +------------------------------ + +Changes to address code analyser scans and two bug fixes +identified while addressing the analysis data. (RIC-673) + + + 2020 October 2; Version 4.3.0 ----------------------------- -Add message types for anomaly detection +Add message types for traffic steering anomaly messages diff --git a/src/rmr/common/include/rmr_agnostic.h b/src/rmr/common/include/rmr_agnostic.h index 2d80a75..419145b 100644 --- a/src/rmr/common/include/rmr_agnostic.h +++ b/src/rmr/common/include/rmr_agnostic.h @@ -285,7 +285,10 @@ static int has_myip( char const* buf, if_addrs_t* list, char sep, int max ); static int uta_tokenise( char* buf, char** tokens, int max, char sep ); static int uta_rmip_tokenise( char* buf, if_addrs_t* iplist, char** toks, int max, char sep ); static char* uta_h2ip( char const* hname ); +#ifdef RTG_PUB +// deprecated funciton -- step 1 of removal static int uta_lookup_rtg( uta_ctx_t* ctx ); +#endif static int uta_has_str( char const* buf, char const* str, char sep, int max ); static char* get_default_ip( if_addrs_t* iplist ); @@ -304,8 +307,8 @@ static int ie_test( void* r, int i_factor, long inserts ); static inline uint64_t build_rt_key( int32_t sub_id, int32_t mtype ); static void collect_things( void* st, void* entry, char const* name, void* thing, void* vthing_list ); static void del_rte( void* st, void* entry, char const* name, void* thing, void* data ); -static endpoint_t* get_meid_owner( route_table_t *rt, char* meid ); -static char* uta_fib( char* fname ); +static endpoint_t* get_meid_owner( route_table_t *rt, char const* meid ); +static char* uta_fib( char const* fname ); static route_table_t* uta_rt_init( ); static route_table_t* uta_rt_clone( route_table_t* srt ); static route_table_t* uta_rt_clone_all( route_table_t* srt ); diff --git a/src/rmr/common/src/mbuf_api.c b/src/rmr/common/src/mbuf_api.c index 029dd74..6dbc71a 100644 --- a/src/rmr/common/src/mbuf_api.c +++ b/src/rmr/common/src/mbuf_api.c @@ -40,6 +40,10 @@ #include "rmr.h" // things the users see #include "rmr_agnostic.h" // agnostic things (must be included before private) +#include "rmr_logging.h" + +//#define BUF_TOOLS_ONLY 1 +#include "tools_static.c" // ---------- some wrappers need explicit copy-in functions, also header field setters ----- @@ -429,7 +433,7 @@ extern unsigned char* rmr_get_src( rmr_mbuf_t* msg, unsigned char* dest ) { if( dest != NULL ) { hdr = msg->header; - strncpy( dest, hdr->src, RMR_MAX_SRC ); + zt_buf_fill( dest, hdr->src, RMR_MAX_SRC ); } return dest; @@ -453,11 +457,11 @@ extern unsigned char* rmr_get_srcip( rmr_mbuf_t* msg, unsigned char* dest ) { hdr = msg->header; if( HDR_VERSION( msg->header ) > 2 ) { // src ip was not present in hdr until ver 3 errno = 0; - strncpy( dest, hdr->srcip, RMR_MAX_SRC ); + zt_buf_fill( dest, hdr->srcip, RMR_MAX_SRC ); rstr = dest; } else { errno = 0; - strncpy( dest, hdr->src, RMR_MAX_SRC ); // reutrn the name:port for old messages + zt_buf_fill( dest, hdr->src, RMR_MAX_SRC ); // reutrn the name:port for old messages rstr = dest; } } diff --git a/src/rmr/common/src/rt_generic_static.c b/src/rmr/common/src/rt_generic_static.c index c196a9a..ea7f01a 100644 --- a/src/rmr/common/src/rt_generic_static.c +++ b/src/rmr/common/src/rt_generic_static.c @@ -4,7 +4,7 @@ Copyright (c) 2019-2020 Nokia Copyright (c) 2018-2020 AT&T Intellectual Property. - Licensed under the Apache License, Version 2.0 (the "License"); + 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 @@ -65,6 +65,10 @@ typedef struct thing_list { /* Dump some stats for an endpoint in the RT. This is generally called to verify endpoints after a table load/change. + + This is called by the for-each mechanism of the symtab and the prototype is + fixe; we don't really use some of the parms, but have dummy references to + keep sonar from complaining. */ static void ep_stats( void* st, void* entry, char const* name, void* thing, void* vcounter ) { int* counter; @@ -76,6 +80,8 @@ static void ep_stats( void* st, void* entry, char const* name, void* thing, void if( (counter = (int *) vcounter) != NULL ) { (*counter)++; + } else { + rmr_vlog( RMR_VL_DEBUG, "ep_stas: nil counter %p %p %p", st, entry, name ); // dummy refs } rmr_vlog_force( RMR_VL_DEBUG, "rt endpoint: target=%s open=%d\n", ep->name, ep->open ); @@ -84,6 +90,8 @@ static void ep_stats( void* st, void* entry, char const* name, void* thing, void /* Called to count meid entries in the table. The meid points to an 'owning' endpoint so we can list what we find + + See note in ep_stats about dummy refs. */ static void meid_stats( void* st, void* entry, char const* name, void* thing, void* vcounter ) { int* counter; @@ -95,6 +103,8 @@ static void meid_stats( void* st, void* entry, char const* name, void* thing, vo if( (counter = (int *) vcounter) != NULL ) { (*counter)++; + } else { + rmr_vlog( RMR_VL_DEBUG, "meid_stas: nil counter %p %p %p", st, entry, name ); // dummy refs } rmr_vlog_force( RMR_VL_DEBUG, "meid=%s owner=%s open=%d\n", name, ep->name, ep->open ); @@ -103,12 +113,15 @@ static void meid_stats( void* st, void* entry, char const* name, void* thing, vo /* Dump counts for an endpoint in the RT. The vid parm is assumed to point to the 'source' information and is added to each message. + + See note above about dummy references. */ static void ep_counts( void* st, void* entry, char const* name, void* thing, void* vid ) { endpoint_t* ep; char* id; if( (ep = (endpoint_t *) thing) == NULL ) { + rmr_vlog( RMR_VL_DEBUG, "ep_counts: nil thing %p %p %p", st, entry, name ); // dummy refs return; } @@ -132,11 +145,12 @@ static void ep_counts( void* st, void* entry, char const* name, void* thing, voi */ static void rte_stats( void* st, void* entry, char const* name, void* thing, void* vcounter ) { int* counter; - rtable_ent_t* rte; // thing is really an rte + rtable_ent_t const* rte; // thing is really an rte int mtype; int sid; if( (rte = (rtable_ent_t *) thing) == NULL ) { + rmr_vlog( RMR_VL_DEBUG, "rte_stats: nil thing %p %p %p", st, entry, name ); // dummy refs return; } @@ -217,7 +231,7 @@ static int send_update_req( uta_ctx_t* pctx, uta_ctx_t* ctx ) { if( smsg != NULL ) { smsg->mtype = RMRRM_REQ_TABLE; smsg->sub_id = 0; - snprintf( smsg->payload, 1024, "%s ts=%ld\n", ctx->my_name, (long) time( NULL ) ); + snprintf( smsg->payload, 1024, "%s ts=%ld\n", ctx->my_name, time( NULL ) ); rmr_vlog( RMR_VL_INFO, "rmr_rtc: requesting table: (%s) whid=%d\n", smsg->payload, ctx->rtg_whid ); smsg->len = strlen( smsg->payload ) + 1; @@ -329,25 +343,38 @@ static char* clip( char* buf ) { */ static char* ensure_nlterm( char* buf ) { char* nb = NULL; - int len = 1; + int len = 0; - nb = buf; - if( buf == NULL || (len = strlen( buf )) < 2 ) { - if( (nb = (char *) malloc( sizeof( char ) * 2 )) != NULL ) { - *nb = '\n'; - *(nb+1) = 0; - } - } else { - if( buf[len-1] != '\n' ) { - rmr_vlog( RMR_VL_WARN, "rmr buf_check: input buffer was not newline terminated (file missing final \\n?)\n" ); - if( (nb = (char *) malloc( sizeof( char ) * (len + 2) )) != NULL ) { - memcpy( nb, buf, len ); - *(nb+len) = '\n'; // insert \n and nil into the two extra bytes we allocated - *(nb+len+1) = 0; - } + if( buf != NULL ) { + len = strlen( buf ); + } - free( buf ); - } + nb = buf; // default to returning original as is + switch( len ) { + case 0: + nb = strdup( "\n" ); + break; + + case 1: + if( *buf != '\n' ) { // not a newline; realloc + rmr_vlog( RMR_VL_WARN, "rmr buf_check: input buffer was not newline terminated (file missing final \\n?)\n" ); + nb = strdup( " \n" ); + *nb = *buf; + free( buf ); + } + break; + + default: + if( buf[len-1] != '\n' ) { // not newline terminated, realloc + rmr_vlog( RMR_VL_WARN, "rmr buf_check: input buffer was not newline terminated (file missing final \\n?)\n" ); + if( (nb = (char *) malloc( sizeof( char ) * (len + 2) )) != NULL ) { + memcpy( nb, buf, len ); + *(nb+len) = '\n'; // insert \n and nil into the two extra bytes we allocated + *(nb+len+1) = 0; + free( buf ); + } + } + break; } return nb; @@ -417,7 +444,7 @@ static rtable_ent_t* uta_add_rte( route_table_t* rt, uint64_t key, int nrrgroups */ static void build_entry( uta_ctx_t* ctx, char* ts_field, uint32_t subid, char* rr_field, int vlevel ) { rtable_ent_t* rte; // route table entry added - char* tok; + char const* tok; int ntoks; uint64_t key = 0; // the symtab key will be mtype or sub_id+mtype char* tokens[128]; @@ -457,7 +484,7 @@ static void build_entry( uta_ctx_t* ctx, char* ts_field, uint32_t subid, char* r } } else { if( DEBUG || (vlevel > 2) ) { - rmr_vlog_force( RMR_VL_DEBUG, "entry not included, sender not matched: %s\n", tokens[1] ); + rmr_vlog_force( RMR_VL_DEBUG, "build entry: ts_entry not of form msg-type,sender: %s\n", ts_field ); } } } @@ -471,7 +498,7 @@ static void build_entry( uta_ctx_t* ctx, char* ts_field, uint32_t subid, char* r */ static void trash_entry( uta_ctx_t* ctx, char* ts_field, uint32_t subid, int vlevel ) { rtable_ent_t* rte; // route table entry to be 'deleted' - char* tok; + char const* tok; int ntoks; uint64_t key = 0; // the symtab key will be mtype or sub_id+mtype char* tokens[128]; @@ -516,7 +543,7 @@ static void trash_entry( uta_ctx_t* ctx, char* ts_field, uint32_t subid, int vle to send messages to. */ static void parse_meid_ar( route_table_t* rtab, char* owner, char* meid_list, int vlevel ) { - char* tok; + char const* tok; int ntoks; char* tokens[128]; int i; @@ -547,7 +574,7 @@ static void parse_meid_ar( route_table_t* rtab, char* owner, char* meid_list, in This function assumes the caller has vetted the pointers as needed. */ static void parse_meid_del( route_table_t* rtab, char* meid_list, int vlevel ) { - char* tok; + char const* tok; int ntoks; char* tokens[128]; int i; @@ -719,7 +746,7 @@ static void parse_rt_rec( uta_ctx_t* ctx, uta_ctx_t* pctx, char* buf, int vleve int ntoks; // number of tokens found in something int ngtoks; int grp; // group number - rtable_ent_t* rte; // route table entry added + rtable_ent_t const* rte; // route table entry added char* tokens[128]; char* tok; // pointer into a token or string char wbuf[1024]; @@ -973,6 +1000,7 @@ static void collect_things( void* st, void* entry, char const* name, void* thing } if( thing == NULL ) { + rmr_vlog_force( RMR_VL_DEBUG, "collect things given nil thing: %p %p %p\n", st, entry, name ); // dummy ref for sonar return; } @@ -1002,6 +1030,7 @@ static void del_rte( void* st, void* entry, char const* name, void* thing, void* int i; if( (rte = (rtable_ent_t *) thing) == NULL ) { + rmr_vlog_force( RMR_VL_DEBUG, "delrte given nil table: %p %p %p\n", st, entry, name ); // dummy ref for sonar return; } @@ -1033,7 +1062,7 @@ static void del_rte( void* st, void* entry, char const* name, void* thing, void* an empty buffer, as opposed to a nil, so the caller can generate defaults or error if an empty/missing file isn't tolerated. */ -static char* uta_fib( char* fname ) { +static char* uta_fib( char const* fname ) { struct stat stats; off_t fsize = 8192; // size of the file off_t nread; // number of bytes read @@ -1110,8 +1139,8 @@ static route_table_t* uta_rt_init( ) { references rte structs. All other spaces use a string key and reference endpoints. */ static route_table_t* rt_clone_space( route_table_t* srt, route_table_t* nrt, int space ) { - endpoint_t* ep; // an endpoint - rtable_ent_t* rte; // a route table entry + endpoint_t* ep; // an endpoint (ignore sonar complaint about const*) + rtable_ent_t* rte; // a route table entry (ignore sonar complaint about const*) void* sst; // source symtab void* nst; // new symtab thing_list_t things; // things from the space to copy @@ -1121,6 +1150,9 @@ static route_table_t* rt_clone_space( route_table_t* srt, route_table_t* nrt, in if( nrt == NULL ) { // make a new table if needed free_on_err = 1; nrt = uta_rt_init(); + if( nrt == NULL ) { + return NULL; + } } if( srt == NULL ) { // source was nil, just give back the new table @@ -1135,7 +1167,7 @@ static route_table_t* rt_clone_space( route_table_t* srt, route_table_t* nrt, in memset( things.names, 0, sizeof( char * ) * things.nalloc ); if( things.things == NULL ) { if( free_on_err ) { - free( nrt->hash ); + rmr_sym_free( nrt->hash ); free( nrt ); nrt = NULL; } @@ -1168,6 +1200,12 @@ static route_table_t* rt_clone_space( route_table_t* srt, route_table_t* nrt, in /* Creates a new route table and then clones the parts of the table which we must keep with each newrt|start. The endpoint and meid entries in the hash must be preserved. + + NOTE: The first call to rt_clone_space() will create the new table and subsequent + calls operate on the new table. The return of subsequent calls can be safely + ignored. There are some code analysers which will claim that there are memory + leaks here; not true as they aren't understanding the logic, just looking at + an ignored return value and assuming it's different than what was passed in. */ static route_table_t* uta_rt_clone( route_table_t* srt ) { endpoint_t* ep; // an endpoint @@ -1179,7 +1217,7 @@ static route_table_t* uta_rt_clone( route_table_t* srt ) { return uta_rt_init(); // no source to clone, just return an empty table } - nrt = rt_clone_space( srt, nrt, RT_NAME_SPACE ); // allocate a new one, add endpoint refs + nrt = rt_clone_space( srt, NULL, RT_NAME_SPACE ); // allocate a new one, add endpoint refs rt_clone_space( srt, nrt, RT_ME_SPACE ); // add meid refs to new return nrt; @@ -1190,10 +1228,12 @@ static route_table_t* uta_rt_clone( route_table_t* srt ) { both endpoints AND the route table entries. Needed to support a partial update where some route table entries will not be deleted if not explicitly in the update and when we are adding/replacing meid references. + + NOTE see note in uta_rt_clone() as it applies here too. */ static route_table_t* uta_rt_clone_all( route_table_t* srt ) { - endpoint_t* ep; // an endpoint - rtable_ent_t* rte; // a route table entry + endpoint_t const* ep; // an endpoint + rtable_ent_t const* rte; // a route table entry route_table_t* nrt = NULL; // new route table int i; @@ -1201,7 +1241,7 @@ static route_table_t* uta_rt_clone_all( route_table_t* srt ) { return uta_rt_init(); // no source to clone, just return an empty table } - nrt = rt_clone_space( srt, nrt, RT_MT_SPACE ); // create new, clone all spaces to it + nrt = rt_clone_space( srt, NULL, RT_MT_SPACE ); // create new, clone all spaces to it rt_clone_space( srt, nrt, RT_NAME_SPACE ); rt_clone_space( srt, nrt, RT_ME_SPACE ); @@ -1288,8 +1328,8 @@ static inline uint64_t build_rt_key( int32_t sub_id, int32_t mtype ) { Given a route table and meid string, find the owner (if known). Returns a pointer to the endpoint struct or nil. */ -static inline endpoint_t* get_meid_owner( route_table_t *rt, char* meid ) { - endpoint_t* ep; // the ep we found in the hash +static inline endpoint_t* get_meid_owner( route_table_t *rt, char const* meid ) { + endpoint_t const* ep; // the ep we found in the hash if( rt == NULL || rt->hash == NULL || meid == NULL || *meid == 0 ) { return NULL; diff --git a/src/rmr/common/src/tools_static.c b/src/rmr/common/src/tools_static.c index df49dd8..a69db35 100644 --- a/src/rmr/common/src/tools_static.c +++ b/src/rmr/common/src/tools_static.c @@ -1,8 +1,8 @@ // :vi sw=4 ts=4 noet: /* ================================================================================== - Copyright (c) 2019 Nokia - Copyright (c) 2018-2019 AT&T Intellectual Property. + Copyright (c) 2019-2020 Nokia + Copyright (c) 2018-2020 AT&T Intellectual Property. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -55,11 +55,47 @@ #include // --- some protos needed for better organisation -------- -int is_this_myip( if_addrs_t* l, char* addr ); +static int is_this_myip( if_addrs_t* l, char* addr ); // ---------------------------------------------------------------------------------- + +/* + A strncpy() replacement that ensures the resulting dest buffer has + a zero (nil) terminator even if the source is longer than the dest + length. + A max of len-1 bytes are copied from src to dest. The copy stops when + a zero (nil) is encountered in the src, or len-1 bytes are copied. + The string is always nil terminated. + The string length is returned. + + It is the responsiblity of the caller to ensure that dest is at + least len bytes in length. + + If either src/dest is invalid (nil) a value of -1 is returned. +*/ +static inline int zt_buf_fill( char* dest, char const* src, int len ) { + char* dp; + char const* sp; + int n; // num moved + + if( dest == NULL && src == NULL ) { + return -1; + } + + dp = dest; + sp = src; + n = 0; + while( *sp && n < len-1 ) { + *(dp++) = *(sp++); + n++; + } + + *dp = 0; + return n; +} + /* Simple tokeniser. Split a null terminated string into tokens recording the pointers in the tokens array provided. Tokens MUST be large enough. Max is @@ -175,6 +211,7 @@ static char* uta_h2ip( char const* hname ) { } +#ifdef RTG_PUB /* Looks for the environment variable RMR_RTG_SVC which we assume to be name[:port], and does a dns lookup on the name. If the env does not have such a variable, we default to @@ -223,6 +260,7 @@ static int uta_lookup_rtg( uta_ctx_t* ctx ) { return ctx->rtg_addr != NULL; } +#endif /* @@ -290,7 +328,7 @@ static int uta_has_str( char const* buf, char const* str, char sep, int max ) { The ENV_BIN_IF environment variable may be either an IP address (v6 must be in square braces), or an interface name (e.g. eth0). */ -if_addrs_t* mk_ip_list( char* port ) { +static if_addrs_t* mk_ip_list( char* port ) { if_addrs_t* l; struct ifaddrs *ifs; // pointer to head struct ifaddrs *ele; // pointer into the list @@ -340,18 +378,18 @@ if_addrs_t* mk_ip_list( char* port ) { fmt = "[%s]:%s"; } } - } - if( *octs ) { - if( (tok = strchr( octs, '%' )) != NULL ) { // for unknown reasons some ip6 addrs have %if-name appended; truncate - *tok = 0; - } - if( l->naddrs < 128 ) { - if( DEBUG ) rmr_vlog( RMR_VL_DEBUG, "capture address: %s: %s\n", ele->ifa_name, octs ); + if( *octs ) { + if( (tok = strchr( octs, '%' )) != NULL ) { // for unknown reasons some ip6 addrs have %if-name appended; truncate + *tok = 0; + } + if( l->naddrs < 128 ) { + if( DEBUG ) rmr_vlog( RMR_VL_DEBUG, "capture address: %s: %s\n", ele->ifa_name, octs ); - snprintf( wbuf, sizeof( wbuf ), fmt, octs, port ); // smash port onto the addr - l->addrs[l->naddrs] = strdup( wbuf ); - l->naddrs++; + snprintf( wbuf, sizeof( wbuf ), fmt, octs, port ); // smash port onto the addr + l->addrs[l->naddrs] = strdup( wbuf ); + l->naddrs++; + } } } } @@ -371,7 +409,7 @@ if_addrs_t* mk_ip_list( char* port ) { do a straight search through the list. We don't expect this to ever be a higly driven functions so not bothering to optimise. */ -int is_this_myip( if_addrs_t* l, char* addr ) { +static int is_this_myip( if_addrs_t* l, char* addr ) { int i; if( l == NULL ) { diff --git a/src/rmr/si/src/rmr_si.c b/src/rmr/si/src/rmr_si.c index 93475f9..2b96424 100644 --- a/src/rmr/si/src/rmr_si.c +++ b/src/rmr/si/src/rmr_si.c @@ -302,7 +302,7 @@ extern rmr_mbuf_t* rmr_rts_msg( void* vctx, rmr_mbuf_t* msg ) { msg->state = RMR_OK; // ensure it is clear before send hold_src = strdup( (char *) ((uta_mhdr_t *)msg->header)->src ); // the dest where we're returning the message to hold_ip = strdup( (char *) ((uta_mhdr_t *)msg->header)->srcip ); // both the src host and src ip - strncpy( (char *) ((uta_mhdr_t *)msg->header)->src, ctx->my_name, RMR_MAX_SRC ); // must overlay the source to be ours + zt_buf_fill( (char *) ((uta_mhdr_t *)msg->header)->src, ctx->my_name, RMR_MAX_SRC ); // must overlay the source to be ours msg = send_msg( ctx, msg, nn_sock, -1 ); if( msg ) { if( ep != NULL ) { @@ -321,8 +321,8 @@ extern rmr_mbuf_t* rmr_rts_msg( void* vctx, rmr_mbuf_t* msg ) { break; } } - strncpy( (char *) ((uta_mhdr_t *)msg->header)->src, hold_src, RMR_MAX_SRC ); // always return original source so rts can be called again - strncpy( (char *) ((uta_mhdr_t *)msg->header)->srcip, hold_ip, RMR_MAX_SRC ); // always return original source so rts can be called again + zt_buf_fill( (char *) ((uta_mhdr_t *)msg->header)->src, hold_src, RMR_MAX_SRC ); // always replace original source & ip so rts can be called again + zt_buf_fill( (char *) ((uta_mhdr_t *)msg->header)->srcip, hold_ip, RMR_MAX_SRC ); msg->flags |= MFL_ADDSRC; // if msg given to send() it must add source } @@ -611,9 +611,6 @@ static void* init( char* uproto_port, int def_msg_size, int flags ) { ctx->max_plen = def_msg_size; } - // we're using a listener to get rtg updates, so we do NOT need this. - //uta_lookup_rtg( ctx ); // attempt to fill in rtg info; rtc will handle missing values/errors - ctx->si_ctx = SIinitialise( SI_OPT_FG ); // FIX ME: si needs to streamline and drop fork/bg stuff if( ctx->si_ctx == NULL ) { rmr_vlog( RMR_VL_CRIT, "unable to initialise SI95 interface\n" ); @@ -652,6 +649,7 @@ static void* init( char* uproto_port, int def_msg_size, int flags ) { } else { if( (gethostname( wbuf, sizeof( wbuf ) )) != 0 ) { rmr_vlog( RMR_VL_CRIT, "rmr_init: cannot determine localhost name: %s\n", strerror( errno ) ); + free_ctx( ctx ); return NULL; } if( (tok = strchr( wbuf, '.' )) != NULL ) { @@ -799,13 +797,6 @@ extern int rmr_get_rcvfd( void* vctx ) { return -1; } -/* - if( (state = nng_getopt_int( ctx->nn_sock, NNG_OPT_RECVFD, &fd )) != 0 ) { - rmr_vlog( RMR_VL_WARN, "rmr cannot get recv fd: %s\n", nng_strerror( state ) ); - return -1; - } -*/ - return uta_ring_getpfd( ctx->mring ); } diff --git a/src/rmr/si/src/rtable_si_static.c b/src/rmr/si/src/rtable_si_static.c index 719053a..f929295 100644 --- a/src/rmr/si/src/rtable_si_static.c +++ b/src/rmr/si/src/rtable_si_static.c @@ -180,6 +180,7 @@ extern endpoint_t* uta_add_ep( route_table_t* rt, rtable_ent_t* rte, char* ep_n if( (rrg->epts = (endpoint_t **) malloc( sizeof( endpoint_t ) * MAX_EP_GROUP )) == NULL ) { rmr_vlog( RMR_VL_WARN, "rmr_add_ep: malloc failed for group endpoint array: group=%d\n", group ); + free( rrg ); return NULL; } memset( rrg->epts, 0, sizeof( endpoint_t ) * MAX_EP_GROUP ); diff --git a/src/rmr/si/src/si95/siaddress.c b/src/rmr/si/src/si95/siaddress.c index 7cec5dd..6533f33 100644 --- a/src/rmr/si/src/si95/siaddress.c +++ b/src/rmr/si/src/si95/siaddress.c @@ -148,6 +148,10 @@ extern int SIaddress( void *src, void **dest, int type ) { int i; int rlen = 0; // return len - len of address struct or string + if( src == NULL || dest == NULL ) { + return rlen; + } + switch( type ) { case AC_TODOT: // convert from a struct to human readable "dotted decimal" addr = (struct sockaddr_in *) src; diff --git a/src/rmr/si/src/si95/sigetname.c b/src/rmr/si/src/si95/sigetname.c index 89cbdf7..fee7199 100644 --- a/src/rmr/si/src/si95/sigetname.c +++ b/src/rmr/si/src/si95/sigetname.c @@ -21,21 +21,30 @@ /* ------------------------------------------------------------------------ Mnemonic: sigetname - Abstract: returns the name of the socket for a given sid - Parms: sid - the socket id as returned by open/listen/connect - Date: 21 July 2003 - Author: E. Scott Daniels + Abstract: returns the name of the socket for a given sid. On failure returns + a nil pointer. + Parms: sid - the socket ID; a fd as returned by open/listen/connect + Date: 21 July 2003 + Author: E. Scott Daniels + + Mods: 30 Oct 2020 - address issues raised by code scan 1654181 ------------------------------------------------------------------------ */ #include "sisetup.h" -extern char *SIgetname( int sid ) { - struct sockaddr oaddr; // pointer to address in TCP binary format - char *buf; +extern char* SIgetname( int sid ) { + struct sockaddr oaddr; // pointer to address in TCP binary format + char *buf = NULL; int len; len = sizeof( oaddr ); - getsockname( sid, &oaddr, &len ); - SIaddress( &oaddr, (void **) &buf, AC_TODOT ); + if( getsockname( sid, &oaddr, &len ) < 0 ) { + return NULL; + } + + if( SIaddress( &oaddr, (void **) &buf, AC_TODOT ) <= 0 ) { + return NULL; + } + return buf; } diff --git a/src/rmr/si/src/si95/sipoll.c b/src/rmr/si/src/si95/sipoll.c index c6a5cd5..d736d14 100644 --- a/src/rmr/si/src/si95/sipoll.c +++ b/src/rmr/si/src/si95/sipoll.c @@ -48,8 +48,8 @@ extern int SIpoll( struct ginfo_blk *gptr, int msdelay ) int fd; // file descriptor for use in this routine int ((*cbptr)()); // pointer to callback routine to call int status = SI_OK; // return status - int addrlen; // length of address from recvfrom call - char *buf; // work buffer pointer + int addrlen = 0; // length of address from recvfrom call + char *buf; // work buffer pointer char ibuf[1025]; int i; // loop index struct tp_blk *tpptr; // pointer at tp stuff diff --git a/src/rmr/si/src/si95/siwait.c b/src/rmr/si/src/si95/siwait.c index 6a72b30..128c1a7 100644 --- a/src/rmr/si/src/si95/siwait.c +++ b/src/rmr/si/src/si95/siwait.c @@ -72,6 +72,7 @@ extern int SIwait( struct ginfo_blk *gptr ) { ibuf = (char *) malloc( 2048 ); if( gptr->flags & GIF_SHUTDOWN ) { // cannot do if we should shutdown + free( ibuf ); return SI_ERROR; // so just get out } diff --git a/src/rmr/si/src/sr_si_static.c b/src/rmr/si/src/sr_si_static.c index 28fb69c..119b88e 100644 --- a/src/rmr/si/src/sr_si_static.c +++ b/src/rmr/si/src/sr_si_static.c @@ -209,8 +209,8 @@ static rmr_mbuf_t* alloc_zcmsg( uta_ctx_t* ctx, rmr_mbuf_t* msg, int size, int s msg->state = state; // fill in caller's state (likely the state of the last operation) msg->flags = MFL_ZEROCOPY; // this is a zerocopy sendable message msg->ring = ctx->zcb_mring; // original msg_free() api doesn't get context so must dup on eaach :( - strncpy( (char *) ((uta_mhdr_t *)msg->header)->src, ctx->my_name, RMR_MAX_SRC ); - strncpy( (char *) ((uta_mhdr_t *)msg->header)->srcip, ctx->my_ip, RMR_MAX_SRC ); + zt_buf_fill( (char *) ((uta_mhdr_t *)msg->header)->src, ctx->my_name, RMR_MAX_SRC ); + zt_buf_fill( (char *) ((uta_mhdr_t *)msg->header)->srcip, ctx->my_ip, RMR_MAX_SRC ); if( DEBUG > 1 ) rmr_vlog( RMR_VL_DEBUG, "alloc_zcmsg mlen=%ld size=%d mpl=%d flags=%02x\n", (long) mlen, size, ctx->max_plen, msg->flags ); @@ -424,6 +424,7 @@ static inline rmr_mbuf_t* realloc_msg( rmr_mbuf_t* old_msg, int tr_len ) { v1hdr = (uta_v1mhdr_t *) old_msg->header; // v1 will work to dig header out of any version switch( ntohl( v1hdr->rmr_ver ) ) { case 1: + v1hdr = nm->header; memcpy( v1hdr, old_msg->header, sizeof( *v1hdr ) ); // copy complete header nm->payload = (void *) v1hdr + sizeof( *v1hdr ); break; @@ -622,8 +623,8 @@ static rmr_mbuf_t* send_msg( uta_ctx_t* ctx, rmr_mbuf_t* msg, int nn_sock, int r tr_len = RMR_TR_LEN( hdr ); // snarf trace len before sending as hdr is invalid after send if( msg->flags & MFL_ADDSRC ) { // buffer was allocated as a receive buffer; must add our source - strncpy( (char *) ((uta_mhdr_t *)msg->header)->src, ctx->my_name, RMR_MAX_SRC ); // must overlay the source to be ours - strncpy( (char *) ((uta_mhdr_t *)msg->header)->srcip, ctx->my_ip, RMR_MAX_SRC ); + zt_buf_fill( (char *) ((uta_mhdr_t *)msg->header)->src, ctx->my_name, RMR_MAX_SRC ); // must overlay the source to be ours + zt_buf_fill( (char *) ((uta_mhdr_t *)msg->header)->srcip, ctx->my_ip, RMR_MAX_SRC ); } if( retries == 0 ) { diff --git a/test/mbuf_api_test.c b/test/mbuf_api_test.c index ac73976..75d8445 100644 --- a/test/mbuf_api_test.c +++ b/test/mbuf_api_test.c @@ -48,6 +48,7 @@ #include "rmr.h" #include "rmr_agnostic.h" +#include "logging.c" #include "mbuf_api.c" #define MSG_VER 3 diff --git a/test/rt_static_test.c b/test/rt_static_test.c index 1647993..15c6dee 100644 --- a/test/rt_static_test.c +++ b/test/rt_static_test.c @@ -123,6 +123,7 @@ static int rt_test( ) { int enu = 0; int state; char *buf; + char *buf2; char* seed_fname; // seed file SOCKET_TYPE nn_sock; // differnt in each transport (nng == struct, SI/Nano == int) rmr_mbuf_t* mbuf; // message for meid route testing @@ -530,6 +531,36 @@ static int rt_test( ) { rt_epcounts( ctx->rtable, "testing" ); rt_epcounts( NULL, "testing" ); + buf = ensure_nlterm( NULL ); + errors += fail_if_nil( buf, "ensure nlterm returned null pointer when given nil ptr" ); + if( buf ) { + errors += fail_not_equal( strlen( buf ), 1, "ensure nlterm returned incorrect length string when given nil pointer" ); + free( buf ); + } + + buf = ensure_nlterm( strdup( "x" ) ); // should return "x\n" + errors += fail_if_nil( buf, "ensure nlterm returned null pointer when given single char string" ); + if( buf ) { + errors += fail_not_equal( strlen( buf ), 2, "ensure nlterm returned incorrect length string when given single char string" ); + free( buf ); + } + + buf = strdup( "x\n" ); + buf2 = ensure_nlterm( buf ); // buffer returned should be the same + if( fail_not_pequal( buf, buf2, "ensure nlterm returned new buffer for one char string with newline" ) ) { + errors++; + free( buf2 ); + } + free( buf ); + + buf = strdup( "Missing our trips to Gloria's for papossas.\n" ); + buf2 = ensure_nlterm( buf ); // buffer returned should be the same + if( fail_not_pequal( buf, buf2, "ensure nlterm returned new buffer for string with newline" ) ) { + errors++; + free( buf2 ); + } + free( buf ); + buf = ensure_nlterm( strdup( "Stand up and cheer!" ) ); // force addition of newline if( buf ) { errors += fail_not_equal( strcmp( buf, "Stand up and cheer!\n" ), 0, "ensure nlterm didn't add newline" ); diff --git a/test/sr_si_static_test.c b/test/sr_si_static_test.c index e3b5a77..0bbd045 100644 --- a/test/sr_si_static_test.c +++ b/test/sr_si_static_test.c @@ -82,7 +82,6 @@ static int sr_si_test() { ctx->max_mlen = ctx->max_plen + sizeof( uta_mhdr_t ); ctx->my_name = strdup( "dummy-test" ); ctx->my_ip = strdup( "30.4.19.86:1111" ); - uta_lookup_rtg( ctx ); gen_rt( ctx ); // forces a static load with some known info since we don't start the rtc() gen_rt( ctx ); // force a second load to test cloning diff --git a/test/test_support.c b/test/test_support.c index 3e12343..fce0449 100644 --- a/test/test_support.c +++ b/test/test_support.c @@ -186,6 +186,24 @@ static int fail_if( int bv, char* what ) { return bv ? BAD : GOOD; } +static int fail_pequal( void* a, void* b, char* what ) { + ts_tests_driven++; + + if( a == b ) { + fprintf( stderr, " %s: pointers were not equal a=%p b=%p\n", what, a, b ); + } + return a == b ? GOOD : BAD; // user may override good/bad so do NOT return a==b directly! +} + +static int fail_not_pequal( void* a, void* b, char* what ) { + ts_tests_driven++; + + if( a != b ) { + fprintf( stderr, " %s: pointers were not equal a=%p b=%p\n", what, a, b ); + } + return a == b ? GOOD : BAD; // user may override good/bad so do NOT return a==b directly! +} + static int fail_not_equal( int a, int b, char* what ) { ts_tests_driven++; diff --git a/test/tools_static_test.c b/test/tools_static_test.c index f1c2ffc..6f95fe5 100644 --- a/test/tools_static_test.c +++ b/test/tools_static_test.c @@ -31,6 +31,31 @@ Date: 3 April 2019 */ +// ------------ zero termianted buffer --------------------------------------------------------- +static int ztbf_test() { + int errors = 0; + char buf[128]; + char* sshort = "Stand up and cheer! Cheer long and loud for old Ohio."; + char* slong = "Now is the time for the bobcat in the forest to make its way back to Court St for a round of pints at the Pub."; + int l1; + + l1 = zt_buf_fill( buf, sshort, 64 ); + errors += fail_not_equal( l1, strlen( sshort ), "zt_buf_fill of short buf returned unexpected len" ); + errors += fail_not_equal( l1, strlen( buf ), "zt_buf_fill of short buf returned len did not match strlen" ); + + l1 = zt_buf_fill( buf, slong, 64 ); + errors += fail_if_equal( l1, strlen( slong ), "zt_buf_fill of long buf returned unexpected len" ); + errors += fail_not_equal( l1, strlen( buf ), "zt_buf_fill of long buf returned len did not match strlen" ); + + l1 = zt_buf_fill( buf, sshort, strlen( sshort ) ); // edge case of exact size + errors += fail_not_equal( l1, strlen( sshort )-1, "zt_buf_fill exact length edge case failed" ); + + l1 = zt_buf_fill( buf, sshort, 1 ); // unrealistic edge case + errors += fail_not_equal( l1, 0, "zt_buf_fill dest len == 1 test failed" ); + + return errors; +} + /* Returns an interface name that is valid in this environment (keeps us from having to know/guess a name to test with. @@ -120,6 +145,8 @@ static int tools_test( ) { free( hname ); // ------------ rtg lookup test ------------------------------------------------------------- +#ifdef KEEP + // pub/sub route table generator is deprecated and should be removed at this point ctx.rtg_port = 0; ctx.rtg_addr = NULL; @@ -140,6 +167,7 @@ static int tools_test( ) { unsetenv( "RMR_RTG_SVC" ); // this should fail as the default name (rtg) will be unknown during testing i = uta_lookup_rtg( &ctx ); errors += fail_if_true( i, "rtg lookup returned that it found something when not expected to" ); +#endif // ------------ my_ip stuff ----------------------------------------------------------------- @@ -200,6 +228,8 @@ static int tools_test( ) { free( ip ); } + errors += ztbf_test(); // test the zero term buffer fill function + // ------------------------------------------------------------------------------------------------- return !!errors; // 1 or 0 regardless of count