From 6511ac74cdc367a94bffeb3743624775acd52c5b Mon Sep 17 00:00:00 2001 From: "E. Scott Daniels" Date: Tue, 27 Aug 2019 10:17:21 -0400 Subject: [PATCH] Address potential error state after good send This change addresses the potential to "hide" the good status of a send in the case where there are multiple round-robin groups for a message type. The send algorithm was modified to eliminate the potential edge case and to report a successful result if the message was accepted for delivery to at least one group. Two issues identified by valgrind (use of a potentially uninitialised variable in a jump) were also corrected. Intermediate commits: Signed-off-by: E. Scott Daniels Add error tag to msg Missing ERR on failure message. Signed-off-by: E. Scott Daniels Prevent NPE in symtab Signed-off-by: E. Scott Daniels Adjust unit tests, fix valgrind inddicated issues Unit tests were added to verify the new get rte function. Valgrind indicated several places where a conditional was using a potentially uninitialised variable; these were fixed. Signed-off-by: E. Scott Daniels Change-Id: I7c0a49713ab3d6fbb8f66e3d286049762433dc58 --- CMakeLists.txt | 2 +- src/rmr/common/include/rmr_agnostic.h | 4 + src/rmr/common/src/rt_generic_static.c | 7 +- src/rmr/common/src/symtab.c | 5 +- src/rmr/nng/include/rmr_nng_private.h | 3 +- src/rmr/nng/src/rmr_nng.c | 7 ++ src/rmr/nng/src/rtable_nng_static.c | 47 ++++++--- src/rmr/nng/src/sr_nng_static.c | 99 +++++++++++------- test/mbuf_api_static_test.c | 10 ++ test/rmr_nng_test.c | 22 ++-- test/rt_nano_static_test.c | 5 +- test/rt_static_test.c | 177 +++++++++++++++++++++++---------- test/sr_nng_static_test.c | 1 - test/test_nng_em.c | 5 +- test/tools_static_test.c | 6 ++ test/unit_test.ksh | 2 + 16 files changed, 278 insertions(+), 124 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5d71d59..e7dc408 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -35,7 +35,7 @@ project( rmr LANGUAGES C ) cmake_minimum_required( VERSION 3.5 ) set( major_version "1" ) # should be automatically populated from git tag later, but until CI process sets a tag we use this -set( minor_version "4" ) +set( minor_version "5" ) set( patch_level "0" ) set( install_root "${CMAKE_INSTALL_PREFIX}" ) diff --git a/src/rmr/common/include/rmr_agnostic.h b/src/rmr/common/include/rmr_agnostic.h index 9000c29..17a5158 100644 --- a/src/rmr/common/include/rmr_agnostic.h +++ b/src/rmr/common/include/rmr_agnostic.h @@ -53,6 +53,7 @@ typedef struct uta_ctx uta_ctx_t; #define ENV_RTG_RAW "RMR_RTG_ISRAW" // if > 0 we expect route table gen messages as raw (not sent from an RMr application) #define ENV_VERBOSE_FILE "RMR_VCTL_FILE" // file where vlevel may be managed for some (non-time critical) functions #define ENV_NAME_ONLY "RMR_SRC_NAMEONLY" // src in message is name only +#define ENV_WARNINGS "RMR_WARNINGS" // if == 1 then we write some, non-performance impacting, warnings #define NO_FLAGS 0 // no flags to pass to a function @@ -63,6 +64,9 @@ typedef struct uta_ctx uta_ctx_t; #define CFL_MTC_ENABLED 0x01 // multi-threaded call is enabled + // context flags +#define CTXFL_WARN 0x01 // ok to warn on stderr for some things that shouldn't happen + // msg buffer flags #define MFL_ZEROCOPY 0x01 // the message is an allocated zero copy message and can be sent. #define MFL_NOALLOC 0x02 // send should NOT allocate a new buffer before returning diff --git a/src/rmr/common/src/rt_generic_static.c b/src/rmr/common/src/rt_generic_static.c index d19c839..816e18a 100644 --- a/src/rmr/common/src/rt_generic_static.c +++ b/src/rmr/common/src/rt_generic_static.c @@ -178,8 +178,9 @@ static char* ensure_nlterm( char* buf ) { memcpy( nb, buf, len ); *(nb+len) = '\n'; // insert \n and nil into the two extra bytes we allocated *(nb+len+1) = 0; - free( buf ); } + + free( buf ); } } @@ -200,7 +201,7 @@ static rtable_ent_t* uta_add_rte( route_table_t* rt, uint64_t key, int nrrgroups } if( (rte = (rtable_ent_t *) malloc( sizeof( *rte ) )) == NULL ) { - fprintf( stderr, "rmr_add_rte: malloc failed for entry\n" ); + fprintf( stderr, "[ERR] rmr_add_rte: malloc failed for entry\n" ); return NULL; } memset( rte, 0, sizeof( *rte ) ); @@ -225,7 +226,7 @@ static rtable_ent_t* uta_add_rte( route_table_t* rt, uint64_t key, int nrrgroups rmr_sym_map( rt->hash, key, rte ); // add to hash using numeric mtype as key - if( DEBUG ) fprintf( stderr, "[DBUG] route table entry created: k=%lx groups=%d\n", key, nrrgroups ); + if( DEBUG ) fprintf( stderr, "[DBUG] route table entry created: k=%llx groups=%d\n", (long long) key, nrrgroups ); return rte; } diff --git a/src/rmr/common/src/symtab.c b/src/rmr/common/src/symtab.c index e96af03..4820ebd 100644 --- a/src/rmr/common/src/symtab.c +++ b/src/rmr/common/src/symtab.c @@ -321,7 +321,10 @@ extern void *rmr_sym_get( void *vtable, const char *name, unsigned int class ) int hv; // hash value of key uint64_t nkey; // numeric key if class 0 - table = (Sym_tab *) vtable; + if( (table = (Sym_tab *) vtable) == NULL ) { + return NULL; + } + sym_tab = table->symlist; if( class ) { diff --git a/src/rmr/nng/include/rmr_nng_private.h b/src/rmr/nng/include/rmr_nng_private.h index 4933f8b..9e93c9f 100644 --- a/src/rmr/nng/include/rmr_nng_private.h +++ b/src/rmr/nng/include/rmr_nng_private.h @@ -108,7 +108,8 @@ static void free_ctx( uta_ctx_t* ctx ); static int uta_link2( endpoint_t* ep ); static int rt_link2_ep( endpoint_t* ep ); static int uta_epsock_byname( route_table_t* rt, char* ep_name, nng_socket* nn_sock ); -static int uta_epsock_rr( route_table_t *rt, uint64_t key, int group, int* more, nng_socket* nn_sock ); +static int uta_epsock_rr( rtable_ent_t* rte, int group, int* more, nng_socket* nn_sock ); +static rtable_ent_t* uta_get_rte( route_table_t *rt, int sid, int mtype, int try_alt ); static inline int xlate_nng_state( int state, int def_state ); diff --git a/src/rmr/nng/src/rmr_nng.c b/src/rmr/nng/src/rmr_nng.c index 3529c46..1b6e1f0 100644 --- a/src/rmr/nng/src/rmr_nng.c +++ b/src/rmr/nng/src/rmr_nng.c @@ -677,6 +677,11 @@ static void* init( char* uproto_port, int max_msg_size, int flags ) { } if( DEBUG ) fprintf( stderr, "[DBUG] default ip address: %s\n", ctx->my_ip ); + if( (tok = getenv( ENV_WARNINGS )) != NULL ) { + if( *tok == '1' ) { + ctx->flags |= CTXFL_WARN; // turn on some warnings (not all, just ones that shouldn't impact performance) + } + } if( (interface = getenv( ENV_BIND_IF )) == NULL ) { @@ -877,6 +882,7 @@ extern rmr_mbuf_t* rmr_mt_rcv( void* vctx, rmr_mbuf_t* mbuf, int max_wait ) { } errno = 0; + state = 0; while( chute->mbuf == NULL && ! errno ) { if( seconds ) { state = sem_timedwait( &chute->barrier, &ts ); // wait for msg or timeout @@ -1004,6 +1010,7 @@ extern rmr_mbuf_t* rmr_mt_call( void* vctx, rmr_mbuf_t* mbuf, int call_id, int m } } + state = 0; errno = 0; while( chute->mbuf == NULL && ! errno ) { if( seconds ) { diff --git a/src/rmr/nng/src/rtable_nng_static.c b/src/rmr/nng/src/rtable_nng_static.c index 875cfe6..da4101b 100644 --- a/src/rmr/nng/src/rtable_nng_static.c +++ b/src/rmr/nng/src/rtable_nng_static.c @@ -244,7 +244,7 @@ static int uta_epsock_byname( route_table_t* rt, char* ep_name, nng_socket* nn_s /* Make a round robin selection within a round robin group for a route table entry. Returns the nanomsg socket if there is a rte for the message - type, and group is defined. Socket is returned via pointer in the parm + key, and group is defined. Socket is returned via pointer in the parm list (nn_sock). The group is the group number to select from. @@ -267,8 +267,8 @@ static int uta_epsock_byname( route_table_t* rt, char* ep_name, nng_socket* nn_s Both of these, in the grand scheme of things, is minor compared to the overhead of grabbing a lock on each call. */ -static int uta_epsock_rr( route_table_t *rt, uint64_t key, int group, int* more, nng_socket* nn_sock ) { - rtable_ent_t* rte; // matching rt entry +static int uta_epsock_rr( rtable_ent_t *rte, int group, int* more, nng_socket* nn_sock ) { + //rtable_ent_t* rte; // matching rt entry endpoint_t* ep; // seected end point int state = FALSE; // processing state int dummy; @@ -286,25 +286,19 @@ static int uta_epsock_rr( route_table_t *rt, uint64_t key, int group, int* more, return FALSE; } - if( rt == NULL ) { - *more = 0; - return FALSE; - } - - if( (rte = rmr_sym_pull( rt->hash, key )) == NULL ) { + if( rte == NULL ) { *more = 0; - //if( DEBUG ) fprintf( stderr, ">>>> rte not found for type = %lu\n", key ); return FALSE; } if( group < 0 || group >= rte->nrrgroups ) { - //if( DEBUG ) fprintf( stderr, ">>>> group out of range: key=%lu group=%d max=%d\n", key, group, rte->nrrgroups ); + //if( DEBUG ) fprintf( stderr, ">>>> group out of range: group=%d max=%d\n", group, rte->nrrgroups ); *more = 0; return FALSE; } if( (rrg = rte->rrgroups[group]) == NULL ) { - //if( DEBUG ) fprintf( stderr, ">>>> rrg not found for type key=%lu\n", key ); + //if( DEBUG ) fprintf( stderr, ">>>> rrg not found for group \n", group ); *more = 0; // groups are inserted contig, so nothing should be after a nil pointer return FALSE; } @@ -316,8 +310,7 @@ static int uta_epsock_rr( route_table_t *rt, uint64_t key, int group, int* more, //if( DEBUG ) fprintf( stderr, ">>>> nothing allocated for the rrg\n" ); return FALSE; - case 1: // exactly one, no rr to deal with and more is not possible even if fanout > 1 - //*nn_sock = rrg->epts[0]->nn_sock; + case 1: // exactly one, no rr to deal with ep = rrg->epts[0]; //if( DEBUG ) fprintf( stderr, ">>>> _rr returning socket with one choice in group \n" ); state = TRUE; @@ -353,4 +346,30 @@ static int uta_epsock_rr( route_table_t *rt, uint64_t key, int group, int* more, return state; } +/* + Finds the rtable entry which matches the key. Returns a nil pointer if + no entry is found. If try_alternate is set, then we will attempt + to find the entry with a key based only on the message type. +*/ +static inline rtable_ent_t* uta_get_rte( route_table_t *rt, int sid, int mtype, int try_alt ) { + uint64_t key; // key is sub id and mtype banged together + rtable_ent_t* rte; // the entry we found + + if( rt == NULL || rt->hash == NULL ) { + return NULL; + } + + key = build_rt_key( sid, mtype ); // first try with a 'full' key + if( ((rte = rmr_sym_pull( rt->hash, key )) != NULL) || ! try_alt ) { // found or not allowed to try the alternate, return what we have + return rte; + } + + if( sid != UNSET_SUBID ) { // not found, and allowed to try alternate; and the sub_id was set + key = build_rt_key( UNSET_SUBID, mtype ); // rebuild key + rte = rmr_sym_pull( rt->hash, key ); // see what we get with this + } + + return rte; +} + #endif diff --git a/src/rmr/nng/src/sr_nng_static.c b/src/rmr/nng/src/sr_nng_static.c index 4a71826..c04690d 100644 --- a/src/rmr/nng/src/sr_nng_static.c +++ b/src/rmr/nng/src/sr_nng_static.c @@ -1,4 +1,4 @@ -// : vi ts=4 sw=4 noet : + // : vi ts=4 sw=4 noet 2 /* ================================================================================== Copyright (c) 2019 Nokia @@ -629,15 +629,15 @@ static rmr_mbuf_t* send_msg( uta_ctx_t* ctx, rmr_mbuf_t* msg, nng_socket nn_sock */ static rmr_mbuf_t* mtosend_msg( void* vctx, rmr_mbuf_t* msg, int max_to ) { + rtable_ent_t* rte; // the route table entry which matches the message key nng_socket nn_sock; // endpoint socket for send uta_ctx_t* ctx; int group; // selected group to get socket for int send_again; // true if the message must be sent again rmr_mbuf_t* clone_m; // cloned message for an nth send int sock_ok; // got a valid socket from round robin select - uint64_t key; // mtype or sub-id/mtype sym table key - int altk_ok = 0; // set true if we can lookup on alternate key if mt/sid lookup fails char* d1; + int ok_sends = 0; // track number of ok sends if( (ctx = (uta_ctx_t *) vctx) == NULL || msg == NULL ) { // bad stuff, bail fast errno = EINVAL; // if msg is null, this is their clue @@ -662,49 +662,76 @@ static rmr_mbuf_t* mtosend_msg( void* vctx, rmr_mbuf_t* msg, int max_to ) { max_to = ctx->send_retries; // convert to retries } + if( (rte = uta_get_rte( ctx->rtable, msg->sub_id, msg->mtype, TRUE )) == NULL ) { // find the entry which matches subid/type allow fallback to type only key + if( ctx->flags & CTXFL_WARN ) { + fprintf( stderr, "[WARN] no endpoint for mtype=%d sub_id=%d\n", msg->mtype, msg->sub_id ); + } + msg->state = RMR_ERR_NOENDPT; + errno = ENXIO; // must ensure it's not eagain + msg->tp_state = errno; + return msg; // caller can resend (maybe) or free + } + send_again = 1; // force loop entry group = 0; // always start with group 0 - - key = build_rt_key( msg->sub_id, msg->mtype ); // route table key to find the entry - if( msg->sub_id != UNSET_SUBID ) { - altk_ok = 1; // if caller's sub-id doesn't hit with mtype, allow mtype only key for retry - } while( send_again ) { - sock_ok = uta_epsock_rr( ctx->rtable, key, group, &send_again, &nn_sock ); // round robin sel epoint; again set if mult groups - if( DEBUG ) fprintf( stderr, "[DBUG] send msg: type=%d again=%d group=%d len=%d sock_ok=%d ak_ok=%d\n", - msg->mtype, send_again, group, msg->len, sock_ok, altk_ok ); - - if( ! sock_ok ) { - if( altk_ok ) { // we can try with the alternate (no sub-id) key - altk_ok = 0; - key = build_rt_key( UNSET_SUBID, msg->mtype ); // build with just the mtype and try again - send_again = 1; // ensure we don't exit the while - continue; - } + sock_ok = uta_epsock_rr( rte, group, &send_again, &nn_sock ); // select endpt from rr group and set again if more groups - msg->state = RMR_ERR_NOENDPT; - errno = ENXIO; // must ensure it's not eagain - msg->tp_state = errno; - return msg; // caller can resend (maybe) or free - } + if( DEBUG ) fprintf( stderr, "[DBUG] mtosend_msg: flgs=0x%04x type=%d again=%d group=%d len=%d sock_ok=%d\n", + msg->flags, msg->mtype, send_again, group, msg->len, sock_ok ); group++; - if( send_again ) { - clone_m = clone_msg( msg ); // must make a copy as once we send this message is not available - if( DEBUG ) fprintf( stderr, "[DBUG] msg cloned: type=%d len=%d\n", msg->mtype, msg->len ); - msg->flags |= MFL_NOALLOC; // send should not allocate a new buffer - msg = send_msg( ctx, msg, nn_sock, max_to ); // do the hard work, msg should be nil on success - /* - if( msg ) { - // error do we need to count successes/errors, how to report some success, esp if last fails? - } - */ + if( sock_ok ) { // with an rte we _should_ always have a socket, but don't bet on it + if( send_again ) { + clone_m = clone_msg( msg ); // must make a copy as once we send this message is not available + if( clone_m == NULL ) { + msg->state = RMR_ERR_SENDFAILED; + errno = ENOMEM; + msg->tp_state = errno; + if( ctx->flags & CTXFL_WARN ) { + fprintf( stderr, "[WARN] unable to clone message for multiple rr-group send\n" ); + } + return msg; + } - msg = clone_m; // clone will be the next to send + if( DEBUG ) fprintf( stderr, "[DBUG] msg cloned: type=%d len=%d\n", msg->mtype, msg->len ); + msg->flags |= MFL_NOALLOC; // keep send from allocating a new message; we have a clone to use + msg = send_msg( ctx, msg, nn_sock, max_to ); // do the hard work, msg should be nil on success + + if( msg != NULL ) { // returned message indicates send error of some sort + rmr_free_msg( msg ); // must ditchone; pick msg so we don't have to unfiddle flags + msg = clone_m; + } else { + ok_sends++; + msg = clone_m; // clone will be the next to send + } + } else { + msg = send_msg( ctx, msg, nn_sock, max_to ); // send the last, and allocate a new buffer; drops the clone if it was + if( DEBUG ) { + if( msg == NULL ) { + fprintf( stderr, "[DBUG] mtosend_msg: send returned nil message!\n" ); + } + } + } } else { - msg = send_msg( ctx, msg, nn_sock, max_to ); // send the last, and allocate a new buffer; drops the clone if it was + if( ctx->flags & CTXFL_WARN ) { + fprintf( stderr, "[WARN] invalid socket for rte, setting no endpoint err: mtype=%d sub_id=%d\n", msg->mtype, msg->sub_id ); + } + msg->state = RMR_ERR_NOENDPT; + errno = ENXIO; + } + } + + if( msg ) { // call functions don't get a buffer back, so a nil check is required + msg->flags &= ~MFL_NOALLOC; // must return with this flag off + if( ok_sends ) { // multiple rr-groups and one was successful; report ok + msg->state = RMR_OK; } + + if( DEBUG ) fprintf( stderr, "[DBUG] final send stats: ok=%d group=%d state=%d\n\n", ok_sends, group, msg->state ); + + msg->tp_state = errno; } return msg; // last message caries the status of last/only send attempt diff --git a/test/mbuf_api_static_test.c b/test/mbuf_api_static_test.c index 1ee2858..936c43a 100644 --- a/test/mbuf_api_static_test.c +++ b/test/mbuf_api_static_test.c @@ -152,10 +152,18 @@ int mbuf_api_test( ) { c = rmr_get_meid( mbuf, NULL ); // should allocate and return c errors += fail_if( c == NULL, "get meid with nil dest pointer (did not allocate a buffer)" ); errors += fail_if( strcmp( c, "test-meid" ) != 0, "did not get expected meid from mbuffer" ); + if( c ) { + free( c ); + c = NULL; + } c = rmr_get_meid( mbuf, c ); errors += fail_if( c == NULL, "get meid with a dest pointer returned no pointer" ); errors += fail_if( strcmp( c, "test-meid" ) != 0, "did not get expected meid from mbuffer" ); + if( c ) { + free( c ); + c = NULL; + } // --- test transaction field access functions --------------------------------------------------- @@ -195,6 +203,7 @@ int mbuf_api_test( ) { i = strcmp( ptr, src_buf ); errors += fail_not_equal( i, 0, "get xaction did not fetch expected string cmp return (a) was not 0" ); free( ptr ); + ptr = NULL; } errno = 999; @@ -231,6 +240,7 @@ int mbuf_api_test( ) { errors += fail_if( i == RMR_OK, "(rv) attempt to copy string to xact with large source buffer" ); + rmr_free_msg( mbuf ); // ------------ trace data tests ---------------------------------------------------------------- // CAUTION: to support standalone mbuf api tests, the underlying buffer reallocation functions are NOT used diff --git a/test/rmr_nng_test.c b/test/rmr_nng_test.c index c9d9a6a..fb12ec2 100644 --- a/test/rmr_nng_test.c +++ b/test/rmr_nng_test.c @@ -96,46 +96,46 @@ static void gen_rt( uta_ctx_t* ctx ); // defined in sr_nng_static_test, but use int main() { int errors = 0; - fprintf( stderr, " starting tool tests\n" ); + fprintf( stderr, "\n starting tool tests\n" ); errors += tools_test(); fprintf( stderr, " error count: %d\n", errors ); - fprintf( stderr, " starting ring tests (%d)\n", errors ); + fprintf( stderr, "\n starting ring tests (%d)\n", errors ); errors += ring_test(); fprintf( stderr, " error count: %d\n", errors ); - fprintf( stderr, " starting symtab tests\n" ); + fprintf( stderr, "\n starting symtab tests\n" ); errors += symtab_test( ); fprintf( stderr, " error count: %d\n", errors ); - fprintf( stderr, " starting rtable tests\n" ); + fprintf( stderr, "\n starting rtable tests\n" ); errors += rt_test(); // route table tests fprintf( stderr, " error count: %d\n", errors ); - fprintf( stderr, " starting RMr API tests\n" ); + fprintf( stderr, "\n starting RMr API tests\n" ); errors += rmr_api_test(); - fprintf( stderr, " run RMr API tests with src name only env var set\n" ); + fprintf( stderr, "\n run RMr API tests with src name only env var set\n" ); setenv( "RMR_SRC_NAMEONLY", "1", 1 ); errors += rmr_api_test(); fprintf( stderr, " error count: %d\n", errors ); - fprintf( stderr, " starting wormhole tests\n" ); + fprintf( stderr, "\n starting wormhole tests\n" ); errors += worm_test(); // test wormhole funcitons fprintf( stderr, " error count: %d\n", errors ); - fprintf( stderr, " starting send/receive tests\n" ); + fprintf( stderr, "\n starting send/receive tests\n" ); errors += sr_nng_test(); // test the send/receive static functions fprintf( stderr, " error count: %d\n", errors ); - fprintf( stderr, " starting mbuf api tests\n" ); + fprintf( stderr, "\n starting mbuf api tests\n" ); errors += mbuf_api_test( ); fprintf( stderr, " error count: %d\n", errors ); if( errors == 0 ) { - fprintf( stderr, " all tests were OK\n" ); + fprintf( stderr, " all tests were OK\n\n" ); } else { - fprintf( stderr, " %d modules reported errors\n", errors ); + fprintf( stderr, " %d modules reported errors\n\n", errors ); } return !!errors; diff --git a/test/rt_nano_static_test.c b/test/rt_nano_static_test.c index 49cff8a..d56f01f 100644 --- a/test/rt_nano_static_test.c +++ b/test/rt_nano_static_test.c @@ -231,7 +231,10 @@ static int rt_test( ) { } } - uta_fib( "no-suhch-file" ); // drive some error checking for coverage + buf = uta_fib( "no-suhch-file" ); // drive some error checking for coverage + if( buf ) { + free( buf ); + } rt = uta_rt_init( ); // get us a route table state = uta_link2( NULL ); diff --git a/test/rt_static_test.c b/test/rt_static_test.c index 13a216b..6d416ef 100644 --- a/test/rt_static_test.c +++ b/test/rt_static_test.c @@ -78,6 +78,19 @@ static int count_entries( route_table_t* rt, int class ) { return counter; } +/* + Builds a route table key. +*/ +static uint64_t build_key( uint32_t mtype, uint32_t sid ) { + uint64_t k; + + k = (uint64_t) sid << 32; + k += mtype; + +fprintf( stderr, " build key: %x %x --> %llx\n", (int) mtype, (int) sid, (long long) k ); + return k; +} + /* This is the main route table test. It sets up a very specific table for testing (not via the generic setup function for other test @@ -87,7 +100,8 @@ static int rt_test( ) { uta_ctx_t* ctx; // context needed to test load static rt route_table_t* rt; // route table route_table_t* crt; // cloned route table - rtable_ent_t* rte; // entry in the table + rtable_ent_t* rte; // route table entries from table + rtable_ent_t* rte2; endpoint_t* ep; // endpoint added int more = 0; // more flag from round robin int errors = 0; // number errors found @@ -99,9 +113,9 @@ static int rt_test( ) { int value; int alt_value; ei_t entries[50]; // end point information - int gcounts[5]; // number of groups in this set - int ecounts[5]; // number of elements per group - int mtypes[5]; // msg type for each group set + int gcounts[7]; // number of groups in this set + int ecounts[7]; // number of elements per group + uint64_t mtypes[7]; // mtype/sid 'key' in the modern RMR world char* tok; char* nxt_tok; int enu = 0; @@ -113,40 +127,62 @@ static int rt_test( ) { setenv( "ENV_VERBOSE_FILE", ".ut_rmr_verbose", 1 ); // allow for verbose code in rtc to be driven i = open( ".ut_rmr_verbose", O_RDWR | O_CREAT, 0644 ); if( i >= 0 ) { - write( 1, "2\n", 2 ); + write( i, "2\n", 2 ); close( i ); } - gcounts[0] = 1; // build entry info -- this is hackish, but saves writing another parser + + /* + The hacky code below calls the necessary rmr functions to create a route table + as though the following were read and parsed by the rmr functions. (This tests + the individual funcitons and avoids writing another parser, so it's not pretty.) + + mse | 0 | 0 | yahoo.com:4561,localhost:4562 + mse | 1 | 0 | localhost:4560,localhost:4568,localhost:4569; localhost:4561,localhost:4562 + mse | 2 | 0 | localhost:4563,localhost:4564 + mse | 3 | 0 | localhost:4565 + mse | 3 | 11 | locahost:5511 + mse | 3 | -1 | localhost:5500 + */ + gcounts[0] = 1; // first entry has 1 group with 2 endpoints; message type 0, sid 0 ecounts[0] = 2; - mtypes[0] = 0; + mtypes[0] = build_key( 0, 0 ); // mtype is now a key of mtype/sid entries[enu].group = 0; entries[enu].ep_name = "yahoo.com:4561"; enu++; // use a dns resolvable name to test that - entries[enu].group = 0; entries[enu].ep_name = "localhost:4562"; enu++; // rest can default to some dummy ip + entries[enu].group = 0; entries[enu].ep_name = "localhost:4562"; enu++; // rest can default to some dummy ip - gcounts[1] = 2; - ecounts[1] = 3; - mtypes[1] = 1; - entries[enu].group = 0; entries[enu].ep_name = "localhost:4561"; enu++; + gcounts[1] = 2; // 2 groups + ecounts[1] = 3; // first has 3 endpoints + mtypes[1] = build_key( 1, 0 ); + entries[enu].group = 0; entries[enu].ep_name = "localhost:4560"; enu++; entries[enu].group = 0; entries[enu].ep_name = "localhost:4568"; enu++; entries[enu].group = 0; entries[enu].ep_name = "localhost:4569"; enu++; - gcounts[2] = 0; // 0 groups means use same rte, this is the next gropup - ecounts[2] = 2; - mtypes[2] = 1; + gcounts[2] = 0; // 0 means use same rte, this is the next group for the entry + ecounts[2] = 2; // 2 endpoints + mtypes[2] = 999; // ignored when appending to previous entry entries[enu].group = 1; entries[enu].ep_name = "localhost:4561"; enu++; entries[enu].group = 1; entries[enu].ep_name = "localhost:4562"; enu++; - gcounts[3] = 1; // 0 groups means use same rte, this is the next gropup - ecounts[3] = 2; - mtypes[3] = 2; + gcounts[3] = 1; // next entry has 1 group + ecounts[3] = 2; // with 2 enpoints + mtypes[3] = build_key( 2, 0 ); entries[enu].group = 0; entries[enu].ep_name = "localhost:4563"; enu++; entries[enu].group = 0; entries[enu].ep_name = "localhost:4564"; enu++; - gcounts[4] = 1; // 0 groups means use same rte, this is the next gropup + gcounts[4] = 1; // three entries for mt==3 with different sids ecounts[4] = 1; - mtypes[4] = 3; - entries[enu].group = 0; entries[enu].ep_name = "localhost:4565"; enu++; + mtypes[4] = build_key( 3, 0 ); + entries[enu].group = 0; entries[enu].ep_name = "localhost:5500"; enu++; + + gcounts[5] = 1; + ecounts[5] = 1; + mtypes[5] = build_key( 3, 11 ); + entries[enu].group = 0; entries[enu].ep_name = "localhost:5511"; enu++; + gcounts[6] = 1; + ecounts[6] = 1; + mtypes[6] = build_key( 3, -1 ); + entries[enu].group = 0; entries[enu].ep_name = "localhost:5512"; enu++; rt = uta_rt_init( ); // get us a route table @@ -178,6 +214,9 @@ static int rt_test( ) { } } + // ----- end hacking together a route table --------------------------------------------------- + + crt = uta_rt_clone( rt ); // clone only the endpoint entries errors += fail_if_nil( crt, "cloned route table" ); if( crt ) { @@ -216,31 +255,70 @@ static int rt_test( ) { //alt_value = uta_epsock_byname( rt, "localhost:4562" ); // we might do a memcmp on the two structs, but for now nothing //errors += fail_if_equal( value, alt_value, "app1/app2 sockets" ); + + // --- test that the get_rte function finds expected keys, and retries to find 'bad' sid attempts for valid mtypes with no sid + rte = uta_get_rte( rt, 0, 1, TRUE ); // s=0 m=1 is defined, so this should return a pointer + errors += fail_if_nil( rte, "get_rte did not return a pointer when s=0 m=1 true given" ); + + rte = uta_get_rte( rt, 0, 1, FALSE ); // the retry shouldn't apply, but ensure it does the righ thing + errors += fail_if_nil( rte, "get_rte did not return a pointer when s=0 m=1 false given" ); + + rte = uta_get_rte( rt, 1000, 1, FALSE ); // s=1000 does not exist for any msg type; should return nil as not allowed to drop sid + errors += fail_not_nil( rte, "get_rte returned a pointer when s=1000 m=1 false given" ); + + rte = uta_get_rte( rt, 1000, 1, TRUE ); // this should also fail as there is no mt==1 sid==-1 defined + errors += fail_not_nil( rte, "get_rte returned a pointer when s=1000 m=1 true given" ); + + rte = uta_get_rte( rt, 0, 3, TRUE ); // mtype sid combo does exist; true/false should not matter + errors += fail_if_nil( rte, "get_rte did not return a pointer when s=0 m=3 true given" ); + + rte2 = uta_get_rte( rt, 11, 3, TRUE ); // same mtype as before, different (valid) group, rte should be different than before + errors += fail_if_nil( rte2, "get_rte did not return a pointer when s=11 m=3 true given" ); + errors += fail_if_true( rte == rte2, "get_rte for mtype==3 and different sids (0 and 11) returned the same rte pointer" ); + + rte2 = uta_get_rte( rt, 0, 3, FALSE ); // since the mtype/sid combo exists, setting false should return the same as before + errors += fail_if_nil( rte2, "get_rte did not return a pointer when s=0 m=3 false given" ); + errors += fail_if_false( rte == rte2, "get_rte did not return same pointer when mtype/sid combo given with different true/false" ); + + rte = uta_get_rte( rt, 12, 3, FALSE ); // this combo does not exist and should fail when alt-key is not allowed (false) + errors += fail_not_nil( rte, "get_rte returned a pointer for s=12, m=3, false" ); + + rte = uta_get_rte( rt, 12, 3, TRUE ); // this should return the entry for the 3/-1 combination + errors += fail_if_nil( rte, "get_rte did not return a pointer for s=12, m=3, true" ); + alt_value = -1; - for( i = 0; i < 10; i++ ) { // round robin return value should be different each time - value = uta_epsock_rr( rt, 1, 0, &more, &nn_sock ); // msg type 1, group 1 - errors += fail_if_equal( value, alt_value, "round robiin sockets with multiple end points" ); - errors += fail_if_false( more, "more for mtype==1" ); - alt_value = value; + rte = uta_get_rte( rt, 0, 1, FALSE ); // get an rte for the next loop + if( rte ) { + for( i = 0; i < 10; i++ ) { // round robin return value should be different each time + value = uta_epsock_rr( rte, 0, &more, &nn_sock ); // msg type 1, group 1 + errors += fail_if_equal( value, alt_value, "round robiin sockets with multiple end points" ); + errors += fail_if_false( more, "more for mtype==1" ); + alt_value = value; + } } more = -1; - for( i = 0; i < 10; i++ ) { // this mtype has only one endpoint, so rr should be same each time - value = uta_epsock_rr( rt, 3, 0, NULL, &nn_sock ); // also test ability to deal properly with nil more pointer - if( i ) { - errors += fail_not_equal( value, alt_value, "round robin sockets with one endpoint" ); - errors += fail_not_equal( more, -1, "more value changed in single group instance" ); + rte = uta_get_rte( rt, 0, 3, FALSE ); // get an rte for the next loop + if( rte ) { + for( i = 0; i < 10; i++ ) { // this mtype has only one endpoint, so rr should be same each time + value = uta_epsock_rr( rte, 0, NULL, &nn_sock ); // also test ability to deal properly with nil more pointer + if( i ) { + errors += fail_not_equal( value, alt_value, "round robin sockets with one endpoint" ); + errors += fail_not_equal( more, -1, "more value changed in single group instance" ); + } + alt_value = value; } - alt_value = value; } - value = uta_epsock_rr( rt, 9, 0, &more, &nn_sock ); // non-existant message type; should return false (0) - errors += fail_not_equal( value, 0, "socket for bad mtype was valid" ); + rte = uta_get_rte( rt, 11, 3, TRUE ); + state = uta_epsock_rr( rte, 22, NULL, NULL ); + errors += fail_if_true( state, "uta_epsock_rr returned bad (non-zero) state when given nil socket pointer" ); uta_rt_clone( NULL ); // verify null parms don't crash things uta_rt_drop( NULL ); - uta_epsock_rr( NULL, 1, 0, &more, &nn_sock ); // drive null case for coverage + uta_epsock_rr( NULL, 0, &more, &nn_sock ); // drive null case for coverage uta_add_rte( NULL, 99, 1 ); + uta_get_rte( NULL, 0, 1000, TRUE ); fprintf( stderr, "[INFO] test: adding end points with nil data; warnings expected\n" ); uta_add_ep( NULL, NULL, "foo", 1 ); @@ -258,25 +336,26 @@ static int rt_test( ) { } uta_rt_drop( rt ); + rt = NULL; if( (ctx = (uta_ctx_t *) malloc( sizeof( uta_ctx_t ) )) != NULL ) { memset( ctx, 0, sizeof( *ctx ) ); if( (seed_fname = getenv( "RMR_SEED_RT" )) != NULL ) { - if( ! (fail_if_nil( rt, "pointer to rt for load test" )) ) { - errors++; - read_static_rt( ctx, 0 ); - unsetenv( "RMR_SEED_RT" ); // unset to test the does not exist condition - read_static_rt( ctx, 0 ); - } else { - fprintf( stderr, " cannot gen rt for load test\n" ); - } - } else { - read_static_rt( ctx, 0 ); // not defined, just drive for that one case + 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 } + + read_static_rt( ctx, 0 ); // drive for not there coverage } - uta_fib( "no-suhch-file" ); // drive some error checking for coverage + + buf = uta_fib( "no-suhch-file" ); // drive some error checking for coverage + if( buf ) { + free( buf ); + } ep = (endpoint_t *) malloc( sizeof( *ep ) ); @@ -286,12 +365,6 @@ static int rt_test( ) { state = uta_link2( ep ); errors += fail_if_true( state, "link2 did not return false when given nil pointers" ); - state = uta_epsock_rr( rt, 122, 0, NULL, NULL ); - errors += fail_if_true( state, "uta_epsock_rr returned bad state when given nil socket pointer" ); - - rt = uta_rt_init( ); // get us a route table - state = uta_epsock_rr( rt, 0, -1, NULL, &nn_sock ); - errors += fail_if_true( state, "uta_epsock_rr returned bad state (true) when given negative group number" ); return !!errors; // 1 or 0 regardless of count } diff --git a/test/sr_nng_static_test.c b/test/sr_nng_static_test.c index 294207c..7c44432 100644 --- a/test/sr_nng_static_test.c +++ b/test/sr_nng_static_test.c @@ -219,6 +219,5 @@ static int sr_nng_test() { rtc( NULL ); // coverage test with nil pointer rtc( ctx ); - return !!errors; } diff --git a/test/test_nng_em.c b/test/test_nng_em.c index 88f5966..8c6a9ca 100644 --- a/test/test_nng_em.c +++ b/test/test_nng_em.c @@ -320,10 +320,10 @@ static int em_nng_sub0_open(nng_socket * s ) { return return_value; } static int em_nng_recv(nng_socket s, void * v, size_t * t, int i ) { - return return_value; } static int em_nng_send( nng_socket s, void* m, int l, int f ) { + free( m ); // we must ditch the message as nng does (or reuses) return return_value; } @@ -365,13 +365,11 @@ static int em_nng_msg_alloc( nng_msg** mp, size_t l ) { */ static void em_nng_free( void* p, size_t l ) { if( p ) { - //fprintf( stderr, ">>>>> not freed: %p\n", p ); free( p ); } } static void em_nng_msg_free( void* p ) { if( p ) { - //fprintf( stderr, ">>>>> not freed: %p\n", p ); free( p ); } } @@ -542,6 +540,7 @@ static int em_nn_recvmsg (int s, struct nn_msghdr *msghdr, int flags ) { } static void em_nn_freemsg( void* ptr ) { + free( ptr ); return; } diff --git a/test/tools_static_test.c b/test/tools_static_test.c index 9e156ec..64da983 100644 --- a/test/tools_static_test.c +++ b/test/tools_static_test.c @@ -107,12 +107,15 @@ static int tools_test( ) { hname = uta_h2ip( "192.168.1.2" ); errors += fail_not_equal( strcmp( hname, "192.168.1.2" ), 0, "h2ip did not return IP address when given address" ); errors += fail_if_nil( hname, "h2ip did not return a pointer" ); + free( hname ); hname = uta_h2ip( "yahoo.com" ); errors += fail_if_nil( hname, "h2ip did not return a pointer" ); + free( hname ); hname = uta_h2ip( "yahoo.com:1234" ); // should ignore the port errors += fail_if_nil( hname, "h2ip did not return a pointer" ); + free( hname ); // ------------ rtg lookup test ------------------------------------------------------------- ctx.rtg_port = 0; @@ -188,6 +191,7 @@ static int tools_test( ) { ip = get_default_ip( if_list ); if( ip ) { + free( ip ); } else { errors += fail_if_nil( ip, "get_defaul_ip returned nil pointer when valid pointer expected" ); } @@ -203,6 +207,8 @@ static int tools_test( ) { } else { errors += fail_if_nil( if_list, "mk_ip_list with a specific interface name returned a nil list" ); } + + free( ip ); } // ------------------------------------------------------------------------------------------------- diff --git a/test/unit_test.ksh b/test/unit_test.ksh index f74e438..214f94d 100755 --- a/test/unit_test.ksh +++ b/test/unit_test.ksh @@ -335,6 +335,8 @@ gen_xml=0 replace_flags=1 # replace ##### in gcov for discounted lines run_nano_tests=0 +export RMR_WARNING=1 # turn on warnings + while [[ $1 == "-"* ]] do case $1 in -- 2.16.6