From 2cdc78c5761185c458d62da9fba7587b6a742062 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Fri, 25 Nov 2022 15:21:03 +0100 Subject: [PATCH] Fixed static compile of util validate FIxed mem-leaks on xpath errors Test: valgrind of confirmed-commit --- lib/src/clixon_xpath.c | 4 +-- lib/src/clixon_xpath_eval.c | 11 +++++-- test/test_confirmed_commit.sh | 59 ++++++++++++++++++++--------------- test/test_netconf.sh | 6 ++-- util/Makefile.in | 4 +-- 5 files changed, 49 insertions(+), 35 deletions(-) diff --git a/lib/src/clixon_xpath.c b/lib/src/clixon_xpath.c index 1bf1dff9..fb820df0 100644 --- a/lib/src/clixon_xpath.c +++ b/lib/src/clixon_xpath.c @@ -611,12 +611,12 @@ xpath_vec_ctx(cxobj *xcur, goto done; if (xp_eval(&xc, xptree, nsc, localonly, xrp) < 0) goto done; + retval = 0; + done: if (xc.xc_nodeset){ free(xc.xc_nodeset); xc.xc_nodeset = NULL; } - retval = 0; - done: if (xptree) xpath_tree_free(xptree); return retval; diff --git a/lib/src/clixon_xpath_eval.c b/lib/src/clixon_xpath_eval.c index 59b9c308..cd104921 100644 --- a/lib/src/clixon_xpath_eval.c +++ b/lib/src/clixon_xpath_eval.c @@ -481,7 +481,7 @@ xp_eval_predicate(xp_ctx *xc, xp_ctx *xrc = NULL; int i; cxobj *x; - xp_ctx *xcc; + xp_ctx *xcc = NULL; if (xs->xs_c0 != NULL){ /* eval previous predicates */ if (xp_eval(xc, xs->xs_c0, nsc, localonly, &xr0) < 0) @@ -522,8 +522,8 @@ xp_eval_predicate(xp_ctx *xc, goto done; if (xp_eval(xcc, xs->xs_c1, nsc, localonly, &xrc) < 0) goto done; - if (xcc) - ctx_free(xcc); + ctx_free(xcc); + xcc = NULL; if (xrc->xc_type == XT_NUMBER){ /* If the result is a number, the result will be converted to true if the number is equal to the context position */ @@ -556,6 +556,8 @@ xp_eval_predicate(xp_ctx *xc, } retval = 0; done: + if (xcc) + ctx_free(xcc); if (xr0) ctx_free(xr0); if (xr1) @@ -932,8 +934,11 @@ xp_relop(xp_ctx *xc1, if (xr->xc_type == XT_BOOL && xr->xc_bool != 0) xr->xc_bool = 1; *xrp = xr; + xr = NULL; retval = 0; done: + if (xr) + ctx_free(xr); return retval; } diff --git a/test/test_confirmed_commit.sh b/test/test_confirmed_commit.sh index a7def9b0..66015da3 100755 --- a/test/test_confirmed_commit.sh +++ b/test/test_confirmed_commit.sh @@ -4,6 +4,9 @@ # TODO: # - privileges drop # - lock check +# Notes: +# 1. May tests without "new" which makes it difficult to debug +# 2. Sleeps are difficult when running valgrind tests when startup times (eg netconf) increase # Magic line must be first in script (see README.md) s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi @@ -100,12 +103,13 @@ function edit_config() { function assert_config_equals() { TARGET="$1" EXPECTED="$2" -# new "get-config: $TARGET" + new "assert_config_equals $TARGET" rpc "<$TARGET/>" "$(data "$EXPECTED")" } # delete all function reset() { + new "reset" rpc "none" "" commit assert_config_equals "candidate" "" @@ -139,11 +143,11 @@ fi new "wait backend" wait_backend -new "Hello check confirm-commit capability" +new "1. Hello check confirm-commit capability" expecteof "$clixon_netconf -f $cfg" 0 "urn:ietf:params:netconf:base:1.1]]>]]>" "urn:ietf:params:netconf:capability:confirmed-commit:1.1" '^$' ################################################################################ -new "netconf ephemeral confirmed-commit rolls back after disconnect" +new "2. netconf ephemeral confirmed-commit rolls back after disconnect" reset edit_config "candidate" "$CONFIGB" assert_config_equals "candidate" "$CONFIGB" @@ -152,7 +156,7 @@ assert_config_equals "running" "" ################################################################################ -new "netconf persistent confirmed-commit" +new "3.netconf persistent confirmed-commit" reset edit_config "candidate" "$CONFIGB" commit "a" @@ -163,27 +167,27 @@ assert_config_equals "running" "$CONFIGBPLUSC" ################################################################################ -new "netconf cancel-commit with invalid persist-id" +new "4. netconf cancel-commit with invalid persist-id" rpc "abc" "applicationinvalid-valueerrora confirmed-commit with the given persist-id was not found" ################################################################################ -new "netconf cancel-commit with valid persist-id" +new "5. netconf cancel-commit with valid persist-id" rpc "ab" "" ################################################################################ -new "netconf persistent confirmed-commit with timeout" +new "6. netconf persistent confirmed-commit with timeout" reset edit_config "candidate" "$CONFIGB" -commit "2abcd" +commit "3abcd" assert_config_equals "running" "$CONFIGB" -sleep 2 +sleep 3 assert_config_equals "running" "" ################################################################################ -new "netconf persistent confirmed-commit with reset timeout" +new "7. netconf persistent confirmed-commit with reset timeout" reset edit_config "candidate" "$CONFIGB" commit "abcde5" @@ -199,7 +203,7 @@ assert_config_equals "running" "" ################################################################################ -new "netconf persistent confirming-commit to epehemeral confirmed-commit should rollback" +new "8. netconf persistent confirming-commit to epehemeral confirmed-commit should rollback" reset edit_config "candidate" "$CONFIGB" commit "10" @@ -209,7 +213,7 @@ assert_config_equals "running" "" ################################################################################ -new "netconf confirming-commit for persistent confirmed-commit with empty persist value" +new "9. netconf confirming-commit for persistent confirmed-commit with empty persist value" reset edit_config "candidate" "$CONFIGB" commit "10" @@ -221,7 +225,7 @@ assert_config_equals "running" "$CONFIGB" # TODO reconsider logic around presence/absence of rollback_db as a signal as dropping permissions may impact ability # to unlink and/or create that file. see clixon_datastore.c#xmldb_delete() and backend_startup.c#startup_mode_startup() -new "backend loads rollback if present at startup" +new "10. backend loads rollback if present at startup" reset edit_config "candidate" "$CONFIGB" commit "" @@ -253,7 +257,7 @@ new "start backend -s init -f $cfg" start_backend -s init -f $cfg ################################################################################ -new "backend loads failsafe at startup if rollback present but cannot be loaded" +new "11. backend loads failsafe at startup if rollback present but cannot be loaded" new "wait backend" wait_backend @@ -301,7 +305,7 @@ wait_backend ################################################################################ -new "ephemeral confirmed-commit survives unrelated ephemeral session disconnect" +new "12. ephemeral confirmed-commit survives unrelated ephemeral session disconnect" reset edit_config "candidate" "$CONFIGB" assert_config_equals "candidate" "$CONFIGB" @@ -309,7 +313,10 @@ assert_config_equals "candidate" "$CONFIGB" # use HELLONO11 which uses older EOM framing sleep 60 | cat <(echo "$HELLONO1160]]>]]>") -| $clixon_netconf -qf $cfg >> /dev/null & PIDS=($(jobs -l % | cut -c 6- | awk '{print $1}')) -assert_config_equals "running" "$CONFIGB" # assert config twice to prove it surives disconnect +if [ $valgrindtest -eq 1 ]; then + sleep 1 +fi +assert_config_equals "running" "$CONFIGB" # assert config twice to prove it survives disconnect assert_config_equals "running" "$CONFIGB" # of ephemeral sessions new "soft kill ${PIDS[0]}" @@ -317,7 +324,7 @@ kill ${PIDS[0]} # kill the while loop above to close STDIN on ################################################################################ -new "cli ephemeral confirmed-commit rolls back after disconnect" +new "13. cli ephemeral confirmed-commit rolls back after disconnect" reset tmppipe=$(mktemp -u) @@ -340,7 +347,7 @@ rm $tmppipe ################################################################################ -new "cli persistent confirmed-commit" +new "14. cli persistent confirmed-commit" reset cat << EOF | clixon_cli -f $cfg >> /dev/null @@ -360,35 +367,35 @@ assert_config_equals "running" "$CONFIGBPLUSC" ################################################################################ -new "cli cancel-commit with invalid persist-id" +new "15. cli cancel-commit with invalid persist-id" expectpart "$($clixon_cli -lo -1 -f $cfg commit persist-id abc cancel)" 255 "a confirmed-commit with the given persist-id was not found" ################################################################################ -new "cli cancel-commit with valid persist-id" +new "16. cli cancel-commit with valid persist-id" expectpart "$($clixon_cli -lo -1 -f $cfg commit persist-id ab cancel)" 0 "^$" assert_config_equals "running" "" ################################################################################ -new "cli cancel-commit with no confirmed-commit in progress" +new "17. cli cancel-commit with no confirmed-commit in progress" expectpart "$($clixon_cli -lo -1 -f $cfg commit persist-id ab cancel)" 255 "no confirmed-commit is in progress" ################################################################################ -new "cli persistent confirmed-commit with timeout" +new "18. cli persistent confirmed-commit with timeout" reset cat << EOF | clixon_cli -f $cfg >> /dev/null set table parameter eth0 -commit confirmed persist abcd 2 +commit confirmed persist abcd 3 EOF assert_config_equals "running" "$CONFIGB" -sleep 2 +sleep 3 assert_config_equals "running" "" ################################################################################ -new "cli persistent confirmed-commit with reset timeout" +new "19. cli persistent confirmed-commit with reset timeout" reset cat << EOF | clixon_cli -f $cfg >> /dev/null set table parameter eth0 @@ -441,7 +448,7 @@ assert_config_equals "running" "$CONFIGBPLUSC" ################################################################################ -new "restconf persistid expect fail" +new "20. restconf persistid expect fail" reset edit_config "candidate" "$CONFIGB" commit "a" diff --git a/test/test_netconf.sh b/test/test_netconf.sh index 660861dc..e4bde339 100755 --- a/test/test_netconf.sh +++ b/test/test_netconf.sh @@ -359,7 +359,9 @@ expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "]]>]]>") -| $clixon_netconf -qf $cfg >> /dev/null & - +if [ $valgrindtest -eq 1 ]; then + sleep 1 +fi PIDS=($(jobs -l % | cut -c 6- | awk '{print $1}')) new "try commit should fail" @@ -373,7 +375,7 @@ sleep 60 | cat <(echo "$HELLONO11" "protocollock-denied[0-9]*errorOperation failed, another session has an ongoing confirmed commit" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "protocollock-denied[0-9]*errorOperation failed, " new "soft kill ${PIDS[0]}" kill ${PIDS[0]} # kill the while loop above to close STDIN on 1st diff --git a/util/Makefile.in b/util/Makefile.in index c60eaf38..1f508fcb 100644 --- a/util/Makefile.in +++ b/util/Makefile.in @@ -155,10 +155,10 @@ clixon_util_regexp: clixon_util_regexp.c $(LIBDEPS) clixon_util_socket: clixon_util_socket.c $(LIBDEPS) $(CC) $(INCLUDES) $(CPPFLAGS) @CFLAGS@ $(LDFLAGS) $^ $(LIBS) -o $@ -clixon_util_validate: clixon_util_validate.c $(LIBDEPS) $(BELIBDEPS) +clixon_util_validate: clixon_util_validate.c $(BELIBDEPS) $(LIBDEPS) $(CC) $(INCLUDES) $(CPPFLAGS) @CFLAGS@ $(LDFLAGS) $^ -l clixon_backend -o $@ $(LIBS) $(BELIBS) -clixon_util_dispatcher: clixon_util_dispatcher.c $(LIBDEPS) $(BELIBDEPS) +clixon_util_dispatcher: clixon_util_dispatcher.c $(BELIBDEPS) $(LIBDEPS) $(CC) $(INCLUDES) $(CPPFLAGS) @CFLAGS@ $(LDFLAGS) $^ -l clixon_backend -o $@ $(LIBS) $(BELIBS) ifdef with_restconf