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 <daniels@research.att.com>
Change-Id: I11a5e89638845b1f31953c16bb584108c0f88850

diff --git a/CHANGES b/CHANGES
index f77ae13..795f4ed 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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