From de63b2996a1144b55ccaaf7ad552170d18478d5c Mon Sep 17 00:00:00 2001 From: "E. Scott Daniels" Date: Thu, 14 Nov 2019 16:03:51 -0500 Subject: [PATCH] Correct bug in payload reallocation function When using the rmr_realloc_payload() function with the copy flag set, it was possible to copy a short length of data, or none at all. This change corrects the miscomputed length to copy. Signed-off-by: E. Scott Daniels Change-Id: I01a98fa66ab69d5f11a792463bc04b73209f098b --- CHANGES | 18 +++++++++++++----- CMakeLists.txt | 2 +- docs/rel-notes.rst | 16 ++++++++++++++++ src/rmr/nng/src/sr_nng_static.c | 4 ++-- test/app_test/ex_rts_receiver.c | 3 +++ test/app_test/rebuild.ksh | 2 +- test/app_test/receiver.c | 5 +++++ test/app_test/run_all.ksh | 2 +- test/app_test/sender.c | 18 +++++++++++++----- test/app_test/v_sender.c | 16 +++++++++------- 10 files changed, 64 insertions(+), 22 deletions(-) diff --git a/CHANGES b/CHANGES index d3b1588..3263a4c 100644 --- a/CHANGES +++ b/CHANGES @@ -2,6 +2,14 @@ API and build change and fix summaries. Doc correctsions and/or changes are not mentioned here; see the commit messages. +2019 November 14; version 1.11.1 + Fix bug in payload reallocation function; correct length of payload + was not always copied. + +2019 November 4; version 1.11.0 + Version bump to move away from the 1.10.* to distinguish between + release A and the trial. + 2019 October 31; version 1.10.2 Provide the means to increase the payload size of a received message without losing the data needed to use the rmr_rts_msg() funciton. @@ -20,7 +28,7 @@ and/or changes are not mentioned here; see the commit messages. Correct application level test issue causing timing problems during jenkins verification testing at command and merge - Handle the NNG connection shutdown status which may now be + Handle the NNG connection shutdown status which may now be generated when a connection throug a proxy is reset. 2019 September 25; version 1.8.2 @@ -28,7 +36,7 @@ and/or changes are not mentioned here; see the commit messages. 2019 September 19; version 1.8.1 Correct missing constant for wrappers. - + 2019 September 19; version 1.8.0 New message types added: RAN_CONNECTED, RAN_RESTARTED, RAN_RECONFIGURED @@ -51,7 +59,7 @@ and/or changes are not mentioned here; see the commit messages. the sender. If this environment variable is not present, the host name (original behaviour) is used. -2019 August 26; version 1.4.0 +2019 August 26; version 1.4.0 New message types were added. 2019 August 16; version 1.3.0 @@ -68,9 +76,9 @@ and/or changes are not mentioned here; see the commit messages. that the transport mechanism might set during send and/or receive operations. C programmes should continue to use errno directly, but in some environments wrappers may not be - able to access errno and this provides the value to them. + able to access errno and this provides the value to them. See the rmr_alloc_msg manual page for more details. - + 2019 August 6; version 1.0.45 (build changes) Support for the Nanomsg transport library has been dropped. The library librmr.* will no longer be included in packages. diff --git a/CMakeLists.txt b/CMakeLists.txt index e005e50..f765f2f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -36,7 +36,7 @@ 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 "11" ) -set( patch_level "0" ) +set( patch_level "1" ) set( install_root "${CMAKE_INSTALL_PREFIX}" ) set( install_inc "include/rmr" ) diff --git a/docs/rel-notes.rst b/docs/rel-notes.rst index 898787b..01adf5b 100644 --- a/docs/rel-notes.rst +++ b/docs/rel-notes.rst @@ -15,6 +15,20 @@ file at the repo root; please refer to that file for a completely up to date listing of API changes. +2019 November 14; version 1.11.1 +-------------------------------------------------------------------------------------------- + +Fix bug in payload reallocation function; correct length of +payload was not always copied. + + +2019 November 4; version 1.11.0 +-------------------------------------------------------------------------------------------- + +Version bump to move away from the 1.10.* to distinguish +between release A and the trial. + + 2019 October 31; version 1.10.2 -------------------------------------------------------------------------------------------- @@ -65,6 +79,7 @@ Correct bug in rmr_torcv_msg() when timeout set to zero (0). Correct missing constant for wrappers. + 2019 September 19; version 1.8.0 -------------------------------------------------------------------------------------------- @@ -128,6 +143,7 @@ directly, but in some environments wrappers may not be able to access errno and this provides the value to them. See the rmr_alloc_msg manual page for more details. + 2019 August 6; version 1.0.45 (build changes) -------------------------------------------------------------------------------------------- diff --git a/src/rmr/nng/src/sr_nng_static.c b/src/rmr/nng/src/sr_nng_static.c index 3146fa2..a4e2410 100644 --- a/src/rmr/nng/src/sr_nng_static.c +++ b/src/rmr/nng/src/sr_nng_static.c @@ -467,10 +467,10 @@ static inline rmr_mbuf_t* realloc_payload( rmr_mbuf_t* old_msg, int payload_len, if( copy ) { // if we need to copy the old payload too if( DEBUG ) fprintf( stderr, "[DBUG] rmr_realloc_payload: copy payload into new message: %d bytes\n", old_psize ); - memcpy( nm->header, omhdr, sizeof( char ) * old_psize ); + memcpy( nm->header, omhdr, sizeof( char ) * (old_psize + RMR_HDR_LEN( omhdr )) ); } else { // just need to copy header if( DEBUG ) fprintf( stderr, "[DBUG] rmr_realloc_payload: copy only header into new message: %d bytes\n", RMR_HDR_LEN( nm->header ) ); - memcpy( nm->header, omhdr, sizeof( char ) * RMR_HDR_LEN( nm->header ) ); + memcpy( nm->header, omhdr, sizeof( char ) * RMR_HDR_LEN( omhdr ) ); } ref_tpbuf( nm, mlen ); // set payload and other pointers in the message to the new tp buffer diff --git a/test/app_test/ex_rts_receiver.c b/test/app_test/ex_rts_receiver.c index 14ae26c..3873c77 100644 --- a/test/app_test/ex_rts_receiver.c +++ b/test/app_test/ex_rts_receiver.c @@ -168,6 +168,9 @@ int main( int argc, char** argv ) { if( ack_count < 1 ) { // 1st ack, so we need to connect, and we'll wait for that sleep( 1 ); } + if( rt_count > 5 ) { // but only pause for max 5sec not 1000s! + rt_count = 5; + } rt_count--; msg = rmr_rts_msg( mrc, msg ); // we don't try to resend if this returns retry } diff --git a/test/app_test/rebuild.ksh b/test/app_test/rebuild.ksh index 1cae81b..bf95b6d 100644 --- a/test/app_test/rebuild.ksh +++ b/test/app_test/rebuild.ksh @@ -47,7 +47,7 @@ echo "$(date) build starts" >&2 git pull # get the up to date code so if run from an old image it's a good test fi cd $build_path - cmake .. + cmake .. -DDEV_PKG=1 make package ) >/tmp/PID$$.log if (( $? != 0 )) diff --git a/test/app_test/receiver.c b/test/app_test/receiver.c index 3cf1498..6f739a5 100644 --- a/test/app_test/receiver.c +++ b/test/app_test/receiver.c @@ -198,6 +198,9 @@ int main( int argc, char** argv ) { while( rt_count > 0 && msg != NULL && msg->state == RMR_ERR_RETRY ) { // to work right in nano we need this :( if( ack_count < 1 ) { // 1st ack, so we need to connect, and we'll wait for that sleep( 1 ); + if( rt_count > 5 ) { + rt_count = 5; // but only for 5sec; not 1000sec! + } } rt_count--; msg = rmr_rts_msg( mrc, msg ); // we don't try to resend if this returns retry @@ -206,6 +209,8 @@ int main( int argc, char** argv ) { ack_count++; } } + + timeout = time( NULL ) + 10; // extend timeout to 10s past last received message } } diff --git a/test/app_test/run_all.ksh b/test/app_test/run_all.ksh index d779d65..1ce5627 100644 --- a/test/app_test/run_all.ksh +++ b/test/app_test/run_all.ksh @@ -62,7 +62,7 @@ echo "----- round robin -----------" run_test run_rr_test.ksh echo "----- rts -------------------" -run_test run_rts_test.ksh -s 20 +run_test run_rts_test.ksh -s 5 -d 100 echo "----- extended payload ------" run_test run_exrts_test.ksh -d 10 -n 1000 diff --git a/test/app_test/sender.c b/test/app_test/sender.c index 015b943..51429b5 100644 --- a/test/app_test/sender.c +++ b/test/app_test/sender.c @@ -214,10 +214,15 @@ int main( int argc, char** argv ) { sbuf = rmr_send_msg( mrc, sbuf ); // retry send until it's good (simple test; real programmes should do better) } if( sbuf->state == RMR_OK ) { + if( successful == 0 ) { + fail_count = 0; // count only after first message goes through + } successful = 1; // indicates only that we sent one successful message, not the current state } else { - if( successful ) { - fail_count++; // count failures after first successful message + fail_count++; // count failures after first successful message + if( !successful && fail_count > 30 ) { + fprintf( stderr, "[FAIL] too many send errors for this test\n" ); + exit( 1 ); } } break; @@ -273,8 +278,9 @@ int main( int argc, char** argv ) { } } - timeout = time( NULL ) + 2; // allow 2 seconds for the pipe to drain from the receiver - while( time( NULL ) < timeout ); + fprintf( stderr, " draining begins\n" ); + timeout = time( NULL ) + 20; // allow 20 seconds for the pipe to drain from the receiver + while( time( NULL ) < timeout ) { if( rcv_fd >= 0 ) { while( (nready = epoll_wait( ep_fd, events, 1, 100 )) > 0 ) { // if something ready to receive (non-blocking check) if( events[0].data.fd == rcv_fd ) { // we only are waiting on 1 thing, so [0] is ok @@ -283,7 +289,7 @@ int main( int argc, char** argv ) { if( rbuf ) { rcvd_count++; rts_ok += vet_received( me, rbuf->payload ); - timeout = time( NULL ) + 2; + timeout = time( NULL ) + 10; // break 10s after last received message } } } @@ -295,6 +301,8 @@ int main( int argc, char** argv ) { } } } + } + fprintf( stderr, " draining finishes\n" ); if( rcvd_count != rts_ok || count != nmsgs ) { pass = 0; diff --git a/test/app_test/v_sender.c b/test/app_test/v_sender.c index 3d53204..f8700b5 100644 --- a/test/app_test/v_sender.c +++ b/test/app_test/v_sender.c @@ -182,13 +182,14 @@ int main( int argc, char** argv ) { sbuf = rmr_send_msg( mrc, sbuf ); // retry send until it's good (simple test; real programmes should do better) } if( sbuf->state == RMR_OK ) { + if( successful == 0 ) { + fail_count = 0; // reset on first good message out + } successful = 1; // indicates only that we sent one successful message, not the current state } else { - if( successful ) { - fail_count++; // count failures after first successful message - } - if( fail_count > 10 ) { - fprintf( stderr, "too many failures\n" ); + fail_count++; // count failures after first successful message + if( ! successful && fail_count > 10 ) { + fprintf( stderr, "[FAIL] too many send failures\n" ); exit( 1 ); } } @@ -243,7 +244,7 @@ int main( int argc, char** argv ) { } } - timeout = time( NULL ) + 2; // allow 2 seconds for the pipe to drain from the receiver + timeout = time( NULL ) + 20; // allow 20 seconds for the pipe to drain from the receiver while( time( NULL ) < timeout ) { if( rcv_fd >= 0 ) { while( (nready = epoll_wait( ep_fd, events, 1, 100 )) > 0 ) { @@ -256,7 +257,7 @@ int main( int argc, char** argv ) { rts_ok += validate_msg( rbuf->payload, rbuf->len ); } - timeout = time( NULL ) + 2; + timeout = time( NULL ) + 10; } } } @@ -264,6 +265,7 @@ int main( int argc, char** argv ) { } if( rcvd_count != rts_ok || count != nmsgs ) { // we might not receive all back if receiver didn't retry, so that is NOT a failure here + fprintf( stderr, " rcvd=%d rts_ok=%d count=%d nmsg=%d\n", rcvd_count, rts_ok, count, nmsgs ); pass = 0; } -- 2.16.6