From 286c2ec04f212b0b87fbac6ca184c86029f7c41b Mon Sep 17 00:00:00 2001
From: Olof hagsand <olof@hagsand.se>
Date: Mon, 18 Feb 2019 16:05:51 +0100
Subject: [PATCH] Added valgrind memory leak tests in testmem.sh for cli and
 netconf

---
 CHANGELOG.md        |  1 +
 apps/cli/cli_main.c |  4 ----
 test/README.md      | 46 ++++++++++++++++++-----------------
 test/clixon.supp    | 29 +++++++++++++++++++++++
 test/lib.sh         | 33 ++++++++++++++++++++++----
 test/mem.sh         | 58 +++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 140 insertions(+), 31 deletions(-)
 create mode 100644 test/clixon.supp
 create mode 100755 test/mem.sh

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6eafdb67..e95772ab 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -135,6 +135,7 @@
 * Replaced all calls to (obsolete) `cli_output` with `fprintf`
 * Added _experimental_ config option `CLICON_CLI_UTF8` default set to 0.
   * CLIgen UTF8 does not work with scrolling and control editing
+* Added valgrind memory leak tests in testmem.sh for cli and netconf
 * Added `make test` from top-level Makefile
 * Added `xml_rootchild_node()` lib function as variant of `xml_rootchild()`
 * Added -o "<option>=<value>" command-line option to all programs: backend, cli, netconf, restconf.
diff --git a/apps/cli/cli_main.c b/apps/cli/cli_main.c
index b3f18000..fba80de1 100644
--- a/apps/cli/cli_main.c
+++ b/apps/cli/cli_main.c
@@ -249,8 +249,6 @@ main(int argc, char **argv)
     int          printgen  = 0;
     int          logclisyntax  = 0;
     int          help = 0;
-    char        *treename = NULL;
-    //    int          len;
     int          logdst = CLICON_LOG_STDERR;
     char        *restarg = NULL; /* what remains after options */
     int          dump_configfile_xml = 0;
@@ -550,8 +548,6 @@ main(int argc, char **argv)
     else
 	retval = 0;
   done:
-    if (treename)
-	free(treename);
     if (restarg)
 	free(restarg);
     // Gets in your face if we log on stderr
diff --git a/test/README.md b/test/README.md
index 422f0006..8c93def9 100644
--- a/test/README.md
+++ b/test/README.md
@@ -1,25 +1,6 @@
 # Clixon tests
 
-This directory contains testing code for clixon and the example
-application. Assumes setup of http daemon as describe under apps/restonf
-- jenkins           Directory w Jenkins specific stuff
-- travis            Directory w Travis specific stuff
-- all.sh            Run through all tests with detailed output, and stop on first error.
-- sum.sh            Run though all tests and print summary
-- mem.sh            Make valgrind 
-- site.sh           Add your site-specific modifications here (see example below)
-- test_nacm.sh      Auth tests using internal NACM
-- test_nacm_ext.sh  Auth tests using external NACM (separate file)
-- test_nacm_protocol.sh  Auth tests for incoming RPC:s
-- test_nacm_module_read.sh  Auth tests for data node read operations
-- test_nacm_module_write.sh  Auth tests for data node write operations
-- test_cli.sh       CLI tests
-- test_netconf.sh   Netconf tests
-- test_restconf.sh  Restconf tests
-- test_yang.sh      Yang tests for constructs not in the example.
-- test_leafref.sh   Yang leafref tests
-- test_datastore.sh Datastore tests
-- and many more...
+## Overview
 
 Tests called 'test_*.sh' and placed in this directory will be
 automatically run as part of the all.sh, sum.sh tests etc. The scripts need to follow some rules to work properly, such as add this magic line as the first command line in the script, which ensures it works well when started from `all.sh`:
@@ -27,6 +8,8 @@ automatically run as part of the all.sh, sum.sh tests etc. The scripts need to f
   s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi
 ```
 
+## Prerequisites
+
 You need to build and install the clixon utility programs before running the tests as some of the tests rely on them:
 ```
   cd util
@@ -38,10 +21,17 @@ You need to start nginx for some of the text. There are instructions in
 * If you run systemd: `sudo systemctl start nginx.service`
 * The [example](../example/README.md) has instructions
 
+## Prefix variable
+
 You can prefix a test with `BE=0` if you want to run your own backend.
 
 To run with debug flags, use the `DBG=<number>` environment variable.
 
+Other variables include:
+* DEMSLEEP Number of seconds to sleep after daemons have started
+
+## Run all tests
+
 You can run an individual test by itself, or run through all tests matching 'test_*.sh' in the directory. Prints test output and stops on first error:
 ```
   all.sh
@@ -49,10 +39,18 @@ You can run an individual test by itself, or run through all tests matching 'tes
 
 Run all tests but continue after errors and only print a summary test output identifying which tests succeeded and which failed:
 ```
-  all.sh summary
+  sum.sh
 ```
 
-Example site.sh file:
+## Memory leak test
+These tests use valgrind to check for memory leaks:
+```
+  mem.sh cli
+  mem.sh netconf
+```
+
+## Site.sh
+You may add your site-specific modifications in a `site.sh` file. Example:
 ```
   # Add your local site specific env variables (or tests) here.
   # Add test to this list that you dont want run
@@ -65,3 +63,7 @@ Example site.sh file:
   IETFRFC=$YANGMODELS/standard/ietf/RFC
 ```
 
+## See also
+
+The [clixon test container](../docker/system) encapsulates the `all.sh` test in a test container. 
+
diff --git a/test/clixon.supp b/test/clixon.supp
new file mode 100644
index 00000000..c2201ab3
--- /dev/null
+++ b/test/clixon.supp
@@ -0,0 +1,29 @@
+{
+   supp2
+   Memcheck:Leak
+   match-leak-kinds: reachable
+   fun:*
+   fun:_dl_new_object
+}
+{
+   supp3
+   Memcheck:Leak
+   match-leak-kinds: reachable
+   fun:*
+   fun:*
+   fun:expand_dynamic_string_token
+}
+{
+   supp44
+   Memcheck:Leak
+   match-leak-kinds: reachable
+   fun:*
+   fun:_dlerror_run
+}
+{
+   supp5
+   Memcheck:Leak
+   match-leak-kinds: reachable
+   fun:*
+   fun:_dl_check_map_versions
+}
diff --git a/test/lib.sh b/test/lib.sh
index 148b439d..c6b2d7c6 100755
--- a/test/lib.sh
+++ b/test/lib.sh
@@ -11,6 +11,7 @@
 # - expectmatch
 
 #set -e
+# : ${A=B} vs : ${A:=B} # colon also checks for NULL
 
 # Testfile (not including path)
 : ${testfile:=$(basename $0)}
@@ -42,6 +43,10 @@ fi
 # Single test. Set by "new"
 testname=
 
+# If valgrindtest use a file to log valgrind output on (checked by new)
+: ${valgrindtest=0}
+: ${valgrindfile=$(mktemp)}
+
 # If set to 0, override starting of clixon_backend in test (you bring your own)
 : ${BE:=1}
 
@@ -52,6 +57,9 @@ testname=
 # eg logging to a file: RCLOG="-l f/www-data/restconf.log"
 : ${RCLOG:=}
 
+# Wait after daemons (backend/restconf) start. Set to 10 if valgrind
+: ${RCWAIT:=1} 
+
 # Parse yangmodels from https://github.com/YangModels/yang
 # Recommended: checkout yangmodels elsewhere in the tree and set the env
 # to that
@@ -66,22 +74,21 @@ testname=
 
 # For memcheck
 #clixon_cli="valgrind --leak-check=full --show-leak-kinds=all clixon_cli"
-clixon_cli=clixon_cli
+: ${clixon_cli:=clixon_cli}
 
 # For memcheck / performance
 #clixon_netconf="valgrind --tool=callgrind clixon_netconf"
 # use kcachegrind to view
 #clixon_netconf="valgrind --leak-check=full --show-leak-kinds=all clixon_netconf"
-clixon_netconf=clixon_netconf
+: ${clixon_netconf:=clixon_netconf}
 
 # How to run restconf stand-alone and using valgrind
 #clixon_restconf="valgrind --trace-children=no --child-silent-after-fork=yes --leak-check=full --show-leak-kinds=all /www-data/clixon_restconf"
-clixon_restconf=/www-data/clixon_restconf
-RCWAIT=1 # Wait after restconf start. Set to 10 if valgrind
+: ${clixon_restconf:=/www-data/clixon_restconf}
 
 # If you test w valgrind, you need to set -F & and sleep 10 when starting
 #clixon_backend="valgrind --leak-check=full --show-leak-kinds=all clixon_backend"
-clixon_backend=clixon_backend
+: ${clixon_backend:=clixon_backend}
 
 dir=/var/tmp/$0
 if [ ! -d $dir ]; then
@@ -109,8 +116,24 @@ err(){
   exit -1 #$testnr
 }
 
+# Test is previous test had valgrind errors if so quit
+checkvalgrind(){
+    if [ -f $valgrindfile ]; then
+	res=$(cat $valgrindfile | grep -e "reachable" -e "lost:"|awk '{print  $4}' | grep -v '^0$')
+	if [ -n "$res" ]; then
+	    >&2 cat $valgrindfile
+	    rm -f $valgrindfile
+	    exit -1	    
+	fi
+	rm -f $valgrindfile
+    fi    
+}
+
 # Increment test number and print a nice string
 new(){
+    if [ $valgrindtest -eq 1 ]; then 
+	checkvalgrind
+    fi
     testnr=`expr $testnr + 1`
     testname=$1
     >&2 echo "Test$testnr [$1]"
diff --git a/test/mem.sh b/test/mem.sh
new file mode 100755
index 00000000..426130ba
--- /dev/null
+++ b/test/mem.sh
@@ -0,0 +1,58 @@
+#!/bin/bash
+# Run valgrind leak test for cli, restconf, netconf or background.
+# Stop on first error
+# 
+if [ $# -ne 1 ]; then 
+    echo "usage: $0 cli|netconf|restconf|backend" # valgrind memleak checks
+    exit -1
+fi
+PROGRAM=$1
+
+valgrindfile=$(mktemp)
+echo "valgrindfile:$valgrindfile"
+
+case "$PROGRAM" in
+    'cli')
+	valgrindtest=1
+	DEMSLEEP=1
+	clixon_cli="/usr/bin/valgrind --leak-check=full --show-leak-kinds=all --suppressions=./clixon.supp --trace-children=no --child-silent-after-fork=yes --log-file=$valgrindfile clixon_cli"
+;;
+    'netconf')
+	valgrindtest=1
+	DEMSLEEP=1
+	clixon_netconf="/usr/bin/valgrind --leak-check=full --show-leak-kinds=all --suppressions=./clixon.supp --trace-children=no --child-silent-after-fork=yes --log-file=$valgrindfile clixon_netconf"
+;;
+#    'backend')
+#	valgrindtest=2
+#	DEMSLEEP=20
+#	clixon_backend="/usr/bin/valgrind --leak-check=full --show-leak-kinds=all --suppressions=./clixon.supp --trace-children=yes --log-file=$valgrindfile clixon_backend"
+#;;
+*)
+    echo "usage: $0 cli|netconf|restconf|backend" # valgrind memleak checks
+    exit -1
+;;
+esac
+
+rm -f 
+
+err=0
+testnr=0
+for test in test*.sh; do
+    testfile=$test
+    DEMSLEEP=$DEMSLEEP . ./$test 
+    errcode=$?
+    if [ $errcode -ne 0 ]; then
+	err=1
+	echo -e "\e[31mError in $test errcode=$errcode"
+	echo -ne "\e[0m"
+	exit $errcode
+    fi
+    if [ $valgrindtest -eq 2 ]; then
+#	sudo cat $valgrindfile
+	sudo checkvalgrind
+#	sudo rm -f $valgrindfile
+    fi
+done
+checkvalgrind
+rm -f $valgrindfile
+