From bf897297010df539909b7638d96557d41fd217b0 Mon Sep 17 00:00:00 2001 From: "E. Scott Daniels" Date: Mon, 24 Feb 2020 10:14:56 -0500 Subject: [PATCH] Fix meid related core dump This change fixes a bug in rtable_nng_static and rtable_si_static modules. The caller's reference to the discovered endpoint was not always being set correctly. The change also corrects the unit test which missed the problem. Issue-ID: RIC-220 Signed-off-by: E. Scott Daniels Change-Id: I11a5e89638845b1f31953c16bb584108c0f88850 --- CHANGES | 3 +++ CMakeLists.txt | 2 +- src/rmr/nng/src/rtable_nng_static.c | 13 ++++++++----- src/rmr/si/src/rtable_si_static.c | 11 +++++++---- test/rt_static_test.c | 3 ++- 5 files changed, 21 insertions(+), 11 deletions(-) diff --git a/CHANGES b/CHANGES index f77ae13..795f4ed 100644 --- a/CHANGES +++ b/CHANGES @@ -2,6 +2,9 @@ API and build change and fix summaries. Doc correctsions and/or changes are not mentioned here; see the commit messages. +2020 February 24; version 3.2.4 + Fix meid bug (RIC-220) causing core dump. + 2020 February 21; version 3.2.3 Add meid routing support to the SI95 interface. diff --git a/CMakeLists.txt b/CMakeLists.txt index 678a408..a7276bb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -38,7 +38,7 @@ cmake_minimum_required( VERSION 3.5 ) set( major_version "3" ) # should be automatically populated from git tag later, but until CI process sets a tag we use this set( minor_version "2" ) -set( patch_level "3" ) +set( patch_level "4" ) set( install_root "${CMAKE_INSTALL_PREFIX}" ) set( install_inc "include/rmr" ) diff --git a/src/rmr/nng/src/rtable_nng_static.c b/src/rmr/nng/src/rtable_nng_static.c index 2cec490..34a1ed1 100644 --- a/src/rmr/nng/src/rtable_nng_static.c +++ b/src/rmr/nng/src/rtable_nng_static.c @@ -429,13 +429,16 @@ static inline char* get_ep_counts( endpoint_t* ep, char* ubuf, int ubuf_len ) { We've been told that the meid is a string, thus we count on it being a nil terminated set of bytes. + + If we return false there is no guarentee that the caller's reference to the + ep is valid or nil. Caller can trus the ep reference only when the return is + true. */ static int epsock_meid( route_table_t *rtable, rmr_mbuf_t* msg, nng_socket* nn_sock, endpoint_t** uepp ) { endpoint_t* ep; // seected end point int state = FALSE; // processing state char* meid; - errno = 0; if( ! nn_sock || msg == NULL || rtable == NULL ) { // missing stuff; bail fast errno = EINVAL; @@ -445,14 +448,14 @@ static int epsock_meid( route_table_t *rtable, rmr_mbuf_t* msg, nng_socket* nn_s meid = ((uta_mhdr_t *) msg->header)->meid; if( (ep = get_meid_owner( rtable, meid )) == NULL ) { - if( uepp != NULL ) { // caller needs refernce to endpoint too - *uepp = NULL; - } - if( DEBUG ) rmr_vlog( RMR_VL_DEBUG, "epsock_meid: no ep in hash for (%s)\n", meid ); return FALSE; } + if( uepp != NULL ) { // ensure ep is returned to the caller + *uepp = ep; + } + state = TRUE; if( ! ep->open ) { // not connected if( ep->addr == NULL ) { // name didn't resolve before, try again diff --git a/src/rmr/si/src/rtable_si_static.c b/src/rmr/si/src/rtable_si_static.c index 6215176..117cfc9 100644 --- a/src/rmr/si/src/rtable_si_static.c +++ b/src/rmr/si/src/rtable_si_static.c @@ -394,6 +394,8 @@ static int uta_epsock_rr( uta_ctx_t* ctx, rtable_ent_t* rte, int group, int* mor We've been told that the meid is a string, thus we count on it being a nil terminated set of bytes. + + If we return false the caller's ep reference may NOT be valid or even nil. */ static int epsock_meid( uta_ctx_t* ctx, route_table_t *rtable, rmr_mbuf_t* msg, int* nn_sock, endpoint_t** uepp ) { endpoint_t* ep; // seected end point @@ -417,11 +419,12 @@ static int epsock_meid( uta_ctx_t* ctx, route_table_t *rtable, rmr_mbuf_t* msg, meid = ((uta_mhdr_t *) msg->header)->meid; - if( (ep = get_meid_owner( rtable, meid )) == NULL ) { - if( uepp != NULL ) { // caller needs refernce to endpoint too - *uepp = NULL; - } + ep = get_meid_owner( rtable, meid ); + if( uepp != NULL ) { // caller needs refernce to endpoint too + *uepp = ep; + } + if( ep == NULL ) { if( DEBUG ) rmr_vlog( RMR_VL_DEBUG, "epsock_meid: no ep in hash for (%s)\n", meid ); return FALSE; } diff --git a/test/rt_static_test.c b/test/rt_static_test.c index ae9709b..9559004 100644 --- a/test/rt_static_test.c +++ b/test/rt_static_test.c @@ -379,13 +379,14 @@ static int rt_test( ) { mbuf->len = 100; rmr_str2meid( mbuf, "meid1" ); // id that we know is in the map + ep = NULL; // force to nil so we see it go non-nil state = epsock_meid( ctx->rtable, mbuf, &nn_sock, &ep ); errors += fail_if_nil( ep, "ep was nil when looking up ep with known meid in message" ); errors += fail_not_equal( state, 1, "state was not true when looking up ep with known meid in message" ); rmr_str2meid( mbuf, "XXXmeid1" ); // id that we know is NOT in the map state = epsock_meid( ctx->rtable, mbuf, &nn_sock, &ep ); - errors += fail_not_nil( ep, "ep was NOT nil when looking ep up with unknown meid in message" ); + // it is NOT a valid check to test ep for nil -- epsock_mied doesn't guarentee ep is set/cleared when state is false errors += fail_not_equal( state, 0, "state was not false when looking up ep with unknown meid in message" ); return !!errors; // 1 or 0 regardless of count -- 2.16.6