All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH 1/2] pec: Convert to the new API
@ 2021-03-15  9:28 Joerg Vehlow
  2021-03-15  9:28 ` [LTP] [PATCH 2/2] pec: Fix multiple event test Joerg Vehlow
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Joerg Vehlow @ 2021-03-15  9:28 UTC (permalink / raw)
  To: ltp

From: Joerg Vehlow <joerg.vehlow@aox-tech.de>

---
 runtest/connectors                            |   2 +-
 testcases/kernel/connectors/Makefile          |   2 -
 testcases/kernel/connectors/connector_test.sh |   9 --
 testcases/kernel/connectors/pec/Makefile      |   2 +-
 testcases/kernel/connectors/pec/README        |  48 --------
 testcases/kernel/connectors/pec/cn_pec.sh     |  67 +++++++++++
 .../kernel/connectors/pec/event_generator.c   |  72 ++++++------
 .../kernel/connectors/pec/pec_listener.c      |  54 ++++-----
 testcases/kernel/connectors/pec/run_pec_test  | 107 ------------------
 9 files changed, 124 insertions(+), 239 deletions(-)
 delete mode 100644 testcases/kernel/connectors/connector_test.sh
 delete mode 100644 testcases/kernel/connectors/pec/README
 create mode 100755 testcases/kernel/connectors/pec/cn_pec.sh
 delete mode 100755 testcases/kernel/connectors/pec/run_pec_test

diff --git a/runtest/connectors b/runtest/connectors
index 6153a98e6..2c7aed474 100644
--- a/runtest/connectors
+++ b/runtest/connectors
@@ -1,2 +1,2 @@
 #DESCRIPTION:Netlink Connector tests
-Connectors connector_test.sh
+cn_pec_sh cn_pec.sh
diff --git a/testcases/kernel/connectors/Makefile b/testcases/kernel/connectors/Makefile
index 04d8a4b91..5f668f419 100644
--- a/testcases/kernel/connectors/Makefile
+++ b/testcases/kernel/connectors/Makefile
@@ -24,6 +24,4 @@ top_srcdir		?= ../../..
 
 include $(top_srcdir)/include/mk/env_pre.mk
 
-INSTALL_TARGETS		:= connector_test.sh
-
 include $(top_srcdir)/include/mk/generic_trunk_target.mk
diff --git a/testcases/kernel/connectors/connector_test.sh b/testcases/kernel/connectors/connector_test.sh
deleted file mode 100644
index f92e10b85..000000000
--- a/testcases/kernel/connectors/connector_test.sh
+++ /dev/null
@@ -1,9 +0,0 @@
-#!/bin/sh
-
-if [ ! -f /proc/net/connector ];then
-	echo "Connectors 0 CONF : system doesn't support execution of the test"
-	exit 32
-fi
-
-$LTPROOT/testcases/bin/run_pec_test
-
diff --git a/testcases/kernel/connectors/pec/Makefile b/testcases/kernel/connectors/pec/Makefile
index fa0aa6828..d9a7f104e 100644
--- a/testcases/kernel/connectors/pec/Makefile
+++ b/testcases/kernel/connectors/pec/Makefile
@@ -24,6 +24,6 @@ top_srcdir		?= ../../../..
 
 include $(top_srcdir)/include/mk/testcases.mk
 
-INSTALL_TARGETS		:= run_pec_test
+INSTALL_TARGETS		:= cn_pec.sh
 
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/connectors/pec/README b/testcases/kernel/connectors/pec/README
deleted file mode 100644
index bf1636475..000000000
--- a/testcases/kernel/connectors/pec/README
+++ /dev/null
@@ -1,48 +0,0 @@
-
-TEST SUITE:
-
-The directory pec contains the tests related to the process event connector.
-
-Process event connector is a netlink connector that reports process events
-to userspace. It sends events such as fork, exec, id change and exit.
-
-There are total 5 testcases.
-
-Note: the test can be run by root only.
-
-TESTS AIM:
-
-The aim of the tests is to test the functionality of process event connector.
-
-FILES DESCRIPTION:
-
-check_connector_enabled.c
-------------------
-This program is used to check if the kernel supports netlink connector.
-
-event_generator.c
-------------------
-This program is used to generate a specified process event (fork, exec, uid,
-gid or exit).
-
-run_pec_test
-------------------
-This script runs all the 5 testcases.
-
-pec_listener.c
-------------------
-This program is used to listen to process events received through the kernel
-connector and print them.
-
-Makefile
-------------------
-The usual makefile for this directory
-
-$LTPROOT/output/pec/*.log
-------------------
-The outputs of event_generator and pec_listeners.
-
-README:
-------------------
-The one you have gone through.
-
diff --git a/testcases/kernel/connectors/pec/cn_pec.sh b/testcases/kernel/connectors/pec/cn_pec.sh
new file mode 100755
index 000000000..98abd50fc
--- /dev/null
+++ b/testcases/kernel/connectors/pec/cn_pec.sh
@@ -0,0 +1,67 @@
+#!/bin/bash
+
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2008 FUJITSU LIMITED
+# Copyright (c) 2021 Joerg Vehlow <joerg.vehlow@aox-tech.de>
+#
+# Author: Li Zefan <lizf@cn.fujitsu.com> 
+#
+# Process event connector is a netlink connector that reports process events
+# to userspace. It sends events such as fork, exec, id change and exit.
+
+TST_SETUP=setup
+TST_TESTFUNC=test
+TST_NEEDS_ROOT=1
+TST_NEEDS_TMPDIR=1
+TST_TEST_DATA="fork exec exit uid gid"
+
+NUM_EVENTS=1
+
+. tst_test.sh
+
+setup()
+{	
+	if ! grep -q cn_proc /proc/net/connector; then
+		tst_brk TCONF "Process Event Connector is not supported or kernel is below 2.6.26"
+		exit 0;
+	fi
+
+	tst_res TINFO "Test process events connector"
+}
+
+test()
+{
+	local event=$2
+	pec_listener >lis_$event.log 2>lis_$event.err &
+	pid=$!
+	# Wait for pec_listener to start listening
+	tst_sleep 100ms
+
+	# Run with absolute path, so the generator can exec itself
+	generator="$(command -v event_generator)"
+	"$generator" -n $NUM_EVENTS -e $event >gen_$event.log 2>gen_$event.err
+	gen_rc=$?
+
+	# Sleep until pec_listener has seen and handled all of the generated events
+	tst_sleep 100ms
+	kill -s SIGINT $pid 2> /dev/null
+	wait $pid
+	lis_rc=$?
+
+	if [ $gen_rc -ne 0 -o ! -s gen_$event.log ]; then
+		tst_brk TBROK "failed to generate process events"
+	fi
+
+	if [ $lis_rc -ne 0 ]; then
+		tst_brk TBROK "failed to execute the listener: $(cat lis_$event.err)"
+	fi
+
+	expected_events="$(cat gen_$event.log)"
+	if grep -q "$expected_events" lis_$event.log; then
+		tst_res TPASS "$event detected by listener"
+	else
+		tst_res TFAIL "$event not detected by listener"
+	fi
+}
+
+tst_run
diff --git a/testcases/kernel/connectors/pec/event_generator.c b/testcases/kernel/connectors/pec/event_generator.c
index cfa6b0dcc..77c6c0515 100644
--- a/testcases/kernel/connectors/pec/event_generator.c
+++ b/testcases/kernel/connectors/pec/event_generator.c
@@ -1,24 +1,13 @@
-/******************************************************************************/
-/*                                                                            */
-/* Copyright (c) 2008 FUJITSU LIMITED                                         */
-/*                                                                            */
-/* This program is free software;  you can redistribute it and/or modify      */
-/* it under the terms of the GNU General Public License as published by       */
-/* the Free Software Foundation; either version 2 of the License, or          */
-/* (at your option) any later version.                                        */
-/*                                                                            */
-/* This program is distributed in the hope that it will be useful,            */
-/* but WITHOUT ANY WARRANTY;  without even the implied warranty of            */
-/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See                  */
-/* the GNU General Public License for more details.                           */
-/*                                                                            */
-/* You should have received a copy of the GNU General Public License          */
-/* along with this program;  if not, write to the Free Software               */
-/* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    */
-/*                                                                            */
-/* Author: Li Zefan <lizf@cn.fujitsu.com>                                     */
-/*                                                                            */
-/******************************************************************************/
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2008 FUJITSU LIMITED
+ * Copyright (c) 2021 Joerg Vehlow <joerg.vehlow@aox-tech.de>
+ * 
+ * Author: Li Zefan <lizf@cn.fujitsu.com>
+ * 
+ * This program is used to generate a specified process event (fork, exec, uid,
+ * gid or exit).
+ */
 
 #include <unistd.h>
 #include <string.h>
@@ -28,7 +17,13 @@
 #include <sys/types.h>
 #include <sys/wait.h>
 
-#include "test.h"
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
+
+extern struct tst_test *tst_test;
+static struct tst_test test = {
+	.forks_child = 1
+};
 
 #define DEFAULT_EVENT_NUM       1
 
@@ -41,11 +36,12 @@ const char *ltp_user = "nobody";
 char **exec_argv;
 
 void (*gen_event) (void);
+static void usage(int status) LTP_ATTRIBUTE_NORETURN;
 
 /*
  * Show the usage
  *
- * @status: the exit status
+ * @param status the exit status
  */
 static void usage(int status)
 {
@@ -61,8 +57,8 @@ static void usage(int status)
  * Generate exec event.
  *
  * We can't just exec nr_event times, because the current process image
- * will be replaced with the new process image, so we use enviroment
- * viriable as event counters, as it will be inherited after exec.
+ * will be replaced with the new process image, so we use environment
+ * variable as event counters, as it will be inherited after exec.
  */
 static void gen_exec(void)
 {
@@ -89,7 +85,8 @@ static void gen_exec(void)
 	printf("exec pid: %d\n", getpid());
 	fflush(stdout);
 
-	execv(exec_argv[0], exec_argv);
+	/* Note: This expects the full path to self in exec_argv[0]! */
+	SAFE_EXECV(exec_argv[0], exec_argv);
 }
 
 /*
@@ -100,13 +97,10 @@ static inline void gen_fork(void)
 	pid_t pid;
 	int status;
 
-	pid = fork();
+	pid = SAFE_FORK();
 	if (pid == 0) {
 		printf("fork parent: %d, child: %d\n", getppid(), getpid());
 		exit(0);
-	} else if (pid < 0) {
-		fprintf(stderr, "fork() failed\n");
-		exit(1);
 	} else {		/* Parent should wait for the child */
 		wait(&status);
 	}
@@ -118,14 +112,14 @@ static inline void gen_fork(void)
 static inline void gen_exit(void)
 {
 	pid_t pid;
+	int status;
 
-	pid = fork();
+	pid = SAFE_FORK();
 	if (pid == 0) {
 		printf("exit pid: %d exit_code: %d\n", getpid(), 0);
 		exit(0);
-	} else if (pid < 0) {
-		fprintf(stderr, "fork() failed\n");
-		exit(1);
+	} else {
+		wait(&status);
 	}
 }
 
@@ -134,7 +128,7 @@ static inline void gen_exit(void)
  */
 static inline void gen_uid(void)
 {
-	setuid(ltp_uid);
+	SAFE_SETUID(ltp_uid);
 	printf("uid pid: %d euid: %d\n", getpid(), ltp_uid);
 }
 
@@ -143,15 +137,15 @@ static inline void gen_uid(void)
  */
 static inline void gen_gid(void)
 {
-	setgid(ltp_gid);
+	SAFE_SETGID(ltp_gid);
 	printf("gid pid: %d egid: %d\n", getpid(), ltp_gid);
 }
 
 /*
  * Read option from user input.
  *
- * @argc: number of arguments
- * @argv: argument list
+ * @param argc number of arguments
+ * @param argv argument list
  */
 static void process_options(int argc, char **argv)
 {
@@ -205,6 +199,8 @@ int main(int argc, char **argv)
 	unsigned long i;
 	struct passwd *ent;
 
+	tst_test = &test;
+
 	process_options(argc, argv);
 
 	ent = getpwnam(ltp_user);
diff --git a/testcases/kernel/connectors/pec/pec_listener.c b/testcases/kernel/connectors/pec/pec_listener.c
index cd653c7b1..64429875c 100644
--- a/testcases/kernel/connectors/pec/pec_listener.c
+++ b/testcases/kernel/connectors/pec/pec_listener.c
@@ -1,24 +1,12 @@
-/******************************************************************************/
-/*                                                                            */
-/* Copyright (c) 2008 FUJITSU LIMITED                                         */
-/*                                                                            */
-/* This program is free software;  you can redistribute it and/or modify      */
-/* it under the terms of the GNU General Public License as published by       */
-/* the Free Software Foundation; either version 2 of the License, or          */
-/* (at your option) any later version.                                        */
-/*                                                                            */
-/* This program is distributed in the hope that it will be useful,            */
-/* but WITHOUT ANY WARRANTY;  without even the implied warranty of            */
-/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See                  */
-/* the GNU General Public License for more details.                           */
-/*                                                                            */
-/* You should have received a copy of the GNU General Public License          */
-/* along with this program;  if not, write to the Free Software               */
-/* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    */
-/*                                                                            */
-/* Author: Li Zefan <lizf@cn.fujitsu.com>                                     */
-/*                                                                            */
-/******************************************************************************/
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2008 FUJITSU LIMITED
+ * 
+ * Author: Li Zefan <lizf@cn.fujitsu.com>
+ * 
+ * This program is used to listen to process events received through the kernel
+ * connector and print them.
+ */
 
 #include <sys/socket.h>
 #include <sys/poll.h>
@@ -70,7 +58,7 @@ struct nlmsghdr *nlhdr;
 /*
  * Handler for signal int. Set exit flag.
  *
- * @signo: the signal number, not used
+ * @param signo the signal number, not used
  */
 static void sigint_handler(int __attribute__ ((unused)) signo)
 {
@@ -80,9 +68,9 @@ static void sigint_handler(int __attribute__ ((unused)) signo)
 /*
  * Send netlink package.
  *
- * @sd: socket descripor
- * @to: the destination sockaddr
- * @cnmsg: the pec control message
+ * @param sd    socket descriptor
+ * @param to    the destination sockaddr
+ * @param cnmsg the pec control message
  */
 static int netlink_send(int sd, struct sockaddr_nl *to, struct cn_msg *cnmsg)
 {
@@ -117,8 +105,8 @@ static int netlink_send(int sd, struct sockaddr_nl *to, struct cn_msg *cnmsg)
 /*
  * Receive package from netlink.
  *
- * @sd: socket descripor
- * @from: source sockaddr
+ * @param sd   socket descriptor
+ * @param from source sockaddr
  */
 static int netlink_recv(int sd, struct sockaddr_nl *from)
 {
@@ -146,9 +134,9 @@ static int netlink_recv(int sd, struct sockaddr_nl *from)
 /*
  * Send control message to PEC.
  *
- * @sd: socket descriptor
- * @to: the destination sockaddr
- * @op: control flag
+ * @param sd socket descriptor
+ * @param to the destination sockaddr
+ * @param op control flag
  */
 static int control_pec(int sd, struct sockaddr_nl *to, enum proc_cn_mcast_op op)
 {
@@ -177,7 +165,7 @@ static int control_pec(int sd, struct sockaddr_nl *to, enum proc_cn_mcast_op op)
 /*
  * Process PEC event.
  *
- * @nlhdr: the netlinke pacakge
+ * @param nlhdr the netlink package
  */
 static void process_event(struct nlmsghdr *nlhdr)
 {
@@ -316,14 +304,14 @@ int main(void)
 		fprintf(stderr, "failed to close PEC listening\n");
 		exit(1);
 	}
-
+ 
 	close(sd);
 	free(nlhdr);
 
 	while (fsync(STDOUT_FILENO) == -1) {
 		if (errno != EIO)
 			break;
-		/* retry once every 10 secodns */
+		/* retry once every 10 seconds */
 		sleep(10);
 	}
 
diff --git a/testcases/kernel/connectors/pec/run_pec_test b/testcases/kernel/connectors/pec/run_pec_test
deleted file mode 100755
index 272948575..000000000
--- a/testcases/kernel/connectors/pec/run_pec_test
+++ /dev/null
@@ -1,107 +0,0 @@
-#!/bin/bash
-
-################################################################################
-##                                                                            ##
-## Copyright (c) 2008 FUJITSU LIMITED                                         ##
-##                                                                            ##
-## This program is free software;  you can redistribute it and#or modify      ##
-## it under the terms of the GNU General Public License as published by       ##
-## the Free Software Foundation; either version 2 of the License, or          ##
-## (at your option) any later version.                                        ##
-##                                                                            ##
-## This program is distributed in the hope that it will be useful, but        ##
-## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
-## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
-## for more details.                                                          ##
-##                                                                            ##
-## You should have received a copy of the GNU General Public License          ##
-## along with this program;  if not, write to the Free Software               ##
-## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    ##
-##                                                                            ##
-## Author: Li Zefan <lizf@cn.fujitsu.com>                                     ##
-##                                                                            ##
-################################################################################
-
-NUM_EVENTS=1
-EVENT_TEST_CASES=( "fork" "exec" "exit" "uid" "gid" )
-
-cd $LTPROOT/testcases/bin
-
-export TCID="pec01"
-export TST_TOTAL=5
-export TST_COUNT=1
-
-exit_status=0
-
-if [ $(id -u) != 0 ]; then
-	tst_brkm TCONF ignored "Test must be run as root"
-	exit 0;
-fi
-
-grep cn_proc /proc/net/connector > /dev/null
-if [ $? -ne 0 ]; then
-	tst_brkm TCONF ignored "Process Event Connector is not supported or kernel is below 2.6.26"
-	exit 0;
-fi
-
-# Run a test case
-#
-# $1: the test number
-# $2: type of event
-run_case()
-{
-	export TST_COUNT=$1
-
-	log="$LTPROOT/output/log"
-	mkdir -p $log 2> /dev/null
-
-	pec_listener > "$log/listener_$1.log" 2>&1 &
-	pid=$!
-	# Wait for pec_listener to start listening
-	sleep $((1*NUM_EVENTS))
-
-	event_generator -e $2 > "$log/generator_$1.log"
-	ret1=$?
-
-	# Sleep until pec_listener has seen and handled all of
-	# the generated events
-	sleep $((1*NUM_EVENTS))
-	kill -s SIGINT $pid 2> /dev/null
-	wait $pid
-	ret2=$?
-
-	if [ $ret1 -ne 0 -o ! -s "$log/generator_$1.log" ]; then
-		tst_resm TFAIL "failed to generate process events"
-		exit_status=1
-		return 1
-	fi
-
-	if [ $ret2 -eq 2 ]; then
-		tst_brkm TCONF NULL "connector may not be supported"
-		exit 0
-	fi
-
-	if [ $ret2 -ne 0 ]; then
-		tst_resm TFAIL "failed to listen process events"
-		exit_status=1
-		return 1
-	fi
-
-	event="`cat $log/generator_$1.log`"
-	cat "$log/listener_$1.log" | grep "$event" > /dev/null
-	if [ $? -eq 0 ]; then
-		tst_resm TPASS "get event - $event"
-	else
-		tst_resm TFAIL "expected event - $event"
-		exit_status=1
-	fi
-}
-
-i=1;
-for CASE in "${EVENT_TEST_CASES[@]}" ; do
-	run_case $i $CASE
-	((i++))
-done
-
-exit $exit_status
-
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [LTP] [PATCH 2/2] pec: Fix multiple event test
  2021-03-15  9:28 [LTP] [PATCH 1/2] pec: Convert to the new API Joerg Vehlow
@ 2021-03-15  9:28 ` Joerg Vehlow
  2021-03-29 18:08   ` Petr Vorel
                     ` (2 more replies)
  2021-03-17  9:34 ` [LTP] [PATCH 1/2] pec: Convert to the new API Petr Vorel
  2021-03-29 17:56 ` Petr Vorel
  2 siblings, 3 replies; 11+ messages in thread
From: Joerg Vehlow @ 2021-03-15  9:28 UTC (permalink / raw)
  To: ltp

From: Joerg Vehlow <joerg.vehlow@aox-tech.de>

The test case for NUM_EVENTS != 1 was not correct at all.
Now the test looks for each event recorded by the generator
in the correct order in the listener's log.

Additionally the test is parameterized, to the the number of generated events
and the default is now set to 10.

Signed-off-by: Joerg Vehlow <joerg.vehlow@aox-tech.de>
---
 testcases/kernel/connectors/pec/cn_pec.sh     | 98 ++++++++++++++++---
 .../kernel/connectors/pec/event_generator.c   | 54 +++++-----
 2 files changed, 113 insertions(+), 39 deletions(-)

diff --git a/testcases/kernel/connectors/pec/cn_pec.sh b/testcases/kernel/connectors/pec/cn_pec.sh
index 98abd50fc..b46ce533f 100755
--- a/testcases/kernel/connectors/pec/cn_pec.sh
+++ b/testcases/kernel/connectors/pec/cn_pec.sh
@@ -9,16 +9,48 @@
 # Process event connector is a netlink connector that reports process events
 # to userspace. It sends events such as fork, exec, id change and exit.
 
+TST_OPTS="n:"
 TST_SETUP=setup
 TST_TESTFUNC=test
+TST_PARSE_ARGS=parse_args
+TST_USAGE=usage
 TST_NEEDS_ROOT=1
 TST_NEEDS_TMPDIR=1
 TST_TEST_DATA="fork exec exit uid gid"
 
-NUM_EVENTS=1
-
 . tst_test.sh
 
+num_events=10
+
+usage()
+{
+	cat << EOF
+usage: $0 [-n <nevents>]
+
+OPTIONS
+-n      The number of evetns to generate per test (default 10)
+EOF
+}
+
+parse_args()
+{
+	case $1 in
+	n) num_events=$2;;
+	esac
+}
+
+free_fd()
+{
+	# Find a free file handle
+	local found
+	for fd in $(seq 200); do
+    	rco="$(true 2>/dev/null >&${fd}; echo $?)"
+    	rci="$(true 2>/dev/null <&${fd}; echo $?)"
+    	[[ "${rco}${rci}" = "11" ]] && found=${fd} && break
+	done
+	echo $found
+}
+
 setup()
 {	
 	if ! grep -q cn_proc /proc/net/connector; then
@@ -32,35 +64,75 @@ setup()
 test()
 {
 	local event=$2
+
+	tst_res TINFO "Testing $2 event (nevents=$num_events)"
+
 	pec_listener >lis_$event.log 2>lis_$event.err &
-	pid=$!
+	lis_pid=$!
 	# Wait for pec_listener to start listening
 	tst_sleep 100ms
 
 	# Run with absolute path, so the generator can exec itself
 	generator="$(command -v event_generator)"
-	"$generator" -n $NUM_EVENTS -e $event >gen_$event.log 2>gen_$event.err
+	"$generator" -n $num_events -e $event >gen_$event.log 2>gen_$event.err
 	gen_rc=$?
 
-	# Sleep until pec_listener has seen and handled all of the generated events
-	tst_sleep 100ms
-	kill -s SIGINT $pid 2> /dev/null
-	wait $pid
+	kill -s SIGINT $lis_pid 2> /dev/null
+	wait $lis_pid
 	lis_rc=$?
 
 	if [ $gen_rc -ne 0 -o ! -s gen_$event.log ]; then
-		tst_brk TBROK "failed to generate process events"
+		tst_brk TBROK "failed to generate process events: $(cat gen_$event.err)"
 	fi
 
 	if [ $lis_rc -ne 0 ]; then
 		tst_brk TBROK "failed to execute the listener: $(cat lis_$event.err)"
 	fi
 
-	expected_events="$(cat gen_$event.log)"
-	if grep -q "$expected_events" lis_$event.log; then
-		tst_res TPASS "$event detected by listener"
+	# The listener writes the same messages as the generator, but it can
+	# also see more events (e.g. for testing exit, a fork is generated).
+	# So: The events generated by the generator have to be in the same order
+	# as the events printed by the listener, but my interleaved with other
+	# messages. To correctly compare them, we have to open both logs
+	# and iterate over both of them at the same time, skipping messages
+	# in the listener log, that are not of interest.
+	# Because some messages may be multiple times in the listener log,
+	# we have to open it only once!
+	# This however does not check, if the listener sees more messages,
+	# than expected.
+
+	fd_act=$(free_fd)
+	[ -z "$fd_act" ] && tst_brk TBROK "No free filehandle found"
+	eval "exec ${fd_act}<lis_$event.log"
+
+	failed=0
+	act_nevents=0
+	while read -r exp; do
+		local found=0
+		act_nevents=$((act_nevents + 1))
+		while read -r <&${fd_act} act; do
+			if [ "$exp" = "$act" ]; then
+				found=1
+				break
+			fi
+		done
+		if [ $found -ne 1 ]; then
+			failed=1
+			tst_res TFAIL "Event was not detected by the event listener: $exp"
+			break
+		fi
+	done <gen_$event.log
+
+	eval "exec ${fd_act}<&-"
+
+	if [ $failed -eq 0 ]; then
+		if [ $act_nevents -ne $num_events ]; then
+			tst_res TBROK "Expected $num_events, but $act_nevents generated"
+		else
+			tst_res TPASS "All events detected"
+		fi
 	else
-		tst_res TFAIL "$event not detected by listener"
+		cat lis_$event.log
 	fi
 }
 
diff --git a/testcases/kernel/connectors/pec/event_generator.c b/testcases/kernel/connectors/pec/event_generator.c
index 77c6c0515..2b1f64f87 100644
--- a/testcases/kernel/connectors/pec/event_generator.c
+++ b/testcases/kernel/connectors/pec/event_generator.c
@@ -94,16 +94,8 @@ static void gen_exec(void)
  */
 static inline void gen_fork(void)
 {
-	pid_t pid;
-	int status;
-
-	pid = SAFE_FORK();
-	if (pid == 0) {
-		printf("fork parent: %d, child: %d\n", getppid(), getpid());
-		exit(0);
-	} else {		/* Parent should wait for the child */
-		wait(&status);
-	}
+	/* The actual fork is already done in main */
+	printf("fork parent: %d, child: %d\n", getppid(), getpid());
 }
 
 /**
@@ -111,16 +103,10 @@ static inline void gen_fork(void)
  */
 static inline void gen_exit(void)
 {
-	pid_t pid;
-	int status;
-
-	pid = SAFE_FORK();
-	if (pid == 0) {
-		printf("exit pid: %d exit_code: %d\n", getpid(), 0);
-		exit(0);
-	} else {
-		wait(&status);
-	}
+	/* exit_signal will always be SIGCHLD, if the process terminates cleanly */
+	printf("exit pid: %d exit_code: %d exit_signal: %d\n", 
+	       getpid(), 0, SIGCHLD);
+	/* exit is called by main already */
 }
 
 /*
@@ -129,7 +115,7 @@ static inline void gen_exit(void)
 static inline void gen_uid(void)
 {
 	SAFE_SETUID(ltp_uid);
-	printf("uid pid: %d euid: %d\n", getpid(), ltp_uid);
+	printf("uid pid: %d euid: %d ruid: %d\n", getpid(), ltp_uid, ltp_uid);
 }
 
 /*
@@ -138,7 +124,7 @@ static inline void gen_uid(void)
 static inline void gen_gid(void)
 {
 	SAFE_SETGID(ltp_gid);
-	printf("gid pid: %d egid: %d\n", getpid(), ltp_gid);
+	printf("gid pid: %d egid: %d rgid: %u\n", getpid(), ltp_gid, ltp_gid);
 }
 
 /*
@@ -211,8 +197,6 @@ int main(int argc, char **argv)
 	ltp_uid = ent->pw_uid;
 	ltp_gid = ent->pw_gid;
 
-	signal(SIGCHLD, SIG_IGN);
-
 	/* special processing for gen_exec, see comments above gen_exec() */
 	if (gen_event == gen_exec) {
 		exec_argv = argv;
@@ -224,8 +208,26 @@ int main(int argc, char **argv)
 	}
 
 	/* other events */
-	for (i = 0; i < nr_event; i++)
-		gen_event();
+	for (i = 0; i < nr_event; i++) {
+		pid_t pid;
+		int status;
+
+		pid = SAFE_FORK();
+		if (pid == 0) {
+			gen_event();
+			exit(0);
+		} else {
+			if (pid != SAFE_WAITPID(pid, &status, 0)) {
+				fprintf(stderr, 
+				        "Child process did not terminate as expected\n");
+				return 1;
+			}
+			if (WEXITSTATUS(status) != 0) {
+				fprintf(stderr, "Child process did not terminate with 0\n");
+				return 1;
+			}
+		}
+	}
 
 	return 0;
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [LTP] [PATCH 1/2] pec: Convert to the new API
  2021-03-15  9:28 [LTP] [PATCH 1/2] pec: Convert to the new API Joerg Vehlow
  2021-03-15  9:28 ` [LTP] [PATCH 2/2] pec: Fix multiple event test Joerg Vehlow
@ 2021-03-17  9:34 ` Petr Vorel
  2021-03-17 10:05   ` Joerg Vehlow
  2021-03-29 17:56 ` Petr Vorel
  2 siblings, 1 reply; 11+ messages in thread
From: Petr Vorel @ 2021-03-17  9:34 UTC (permalink / raw)
  To: ltp

Hi Joerg,

thanks for rewriting this, I'll try to have look soon.

BTW: would it make sense to rename runtest/connectors to runtest/netlink
and move code from testcases/kernel/connectors/ to testcases/kernel/netlink/?

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [LTP] [PATCH 1/2] pec: Convert to the new API
  2021-03-17  9:34 ` [LTP] [PATCH 1/2] pec: Convert to the new API Petr Vorel
@ 2021-03-17 10:05   ` Joerg Vehlow
  2021-03-17 11:47     ` Petr Vorel
  0 siblings, 1 reply; 11+ messages in thread
From: Joerg Vehlow @ 2021-03-17 10:05 UTC (permalink / raw)
  To: ltp

Hi Petr,

On 3/17/2021 10:34 AM, Petr Vorel wrote:
> BTW: would it make sense to rename runtest/connectors to runtest/netlink
> and move code from testcases/kernel/connectors/ to testcases/kernel/netlink/?
Maybe kernel/netlink/connector/. Connector is an abstraction on top of 
netlink with a slightly
different interface than pure netlink (eg. cn_msg instead of nlmsghdr).
There are only very few other modules, that use this interface at the 
moment (MS HyperV, MD,? uvesafb? and dallas' 1-wire).

The question is: Is the interface the correct think to categorize by? If 
the interface (i.e. netlink) is used for categorization, then some of 
the crypto tests should also be in this netlink category.

J?rg

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [LTP] [PATCH 1/2] pec: Convert to the new API
  2021-03-17 10:05   ` Joerg Vehlow
@ 2021-03-17 11:47     ` Petr Vorel
  0 siblings, 0 replies; 11+ messages in thread
From: Petr Vorel @ 2021-03-17 11:47 UTC (permalink / raw)
  To: ltp

Hi J?rg,

> Hi Petr,

> On 3/17/2021 10:34 AM, Petr Vorel wrote:
> > BTW: would it make sense to rename runtest/connectors to runtest/netlink
> > and move code from testcases/kernel/connectors/ to testcases/kernel/netlink/?
> Maybe kernel/netlink/connector/. Connector is an abstraction on top of
+1

> netlink with a slightly
> different interface than pure netlink (eg. cn_msg instead of nlmsghdr).
> There are only very few other modules, that use this interface at the moment
> (MS HyperV, MD,? uvesafb? and dallas' 1-wire).
I'd still put it into netlink and try to put there more general netlink
subsystem tests. My objection is that runtest/connectors has only single test
and connectors is very generic name.

> The question is: Is the interface the correct think to categorize by? If the
> interface (i.e. netlink) is used for categorization, then some of the crypto
> tests should also be in this netlink category.
Thus maybe add some of relevant tests also into runtest/netlink? We shave some
kind of duplicity (see runtest/cve, it contains some tests which are also in
runtest/syscalls). That would also justify creating runtest/netlink (because
having just single test for legacy connector interface in runtest/netlink does
not sound good to me).

Kind regards,
Petr

> J?rg

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [LTP] [PATCH 1/2] pec: Convert to the new API
  2021-03-15  9:28 [LTP] [PATCH 1/2] pec: Convert to the new API Joerg Vehlow
  2021-03-15  9:28 ` [LTP] [PATCH 2/2] pec: Fix multiple event test Joerg Vehlow
  2021-03-17  9:34 ` [LTP] [PATCH 1/2] pec: Convert to the new API Petr Vorel
@ 2021-03-29 17:56 ` Petr Vorel
  2 siblings, 0 replies; 11+ messages in thread
From: Petr Vorel @ 2021-03-29 17:56 UTC (permalink / raw)
  To: ltp

Hi Joerg,

> +++ b/testcases/kernel/connectors/pec/cn_pec.sh
...
> +	# Run with absolute path, so the generator can exec itself
> +	generator="$(command -v event_generator)"
> +	"$generator" -n $NUM_EVENTS -e $event >gen_$event.log 2>gen_$event.err
> +	gen_rc=$?
...
I suggest to use "p" exec family - event_generator will be always installed into
$LTPROOT/testcases/bin => drop command. And for development (running without
make install), there should be always also test directory in PATH.

> +++ b/testcases/kernel/connectors/pec/event_generator.c
...
> -#include "test.h"
> +#define TST_NO_DEFAULT_MAIN
> +#include "tst_test.h"

> -	execv(exec_argv[0], exec_argv);
> +	/* Note: This expects the full path to self in exec_argv[0]! */
> +	SAFE_EXECV(exec_argv[0], exec_argv);
We don't have this yet. But I've sent patch [1] which adds SAFE_EXECVP()
because of expecting command to be in PATH.

There are more changes suggested:
* use ROD where possible (cannot be used for wait, because it's not a binary but
  a shell command)
* remove useless exit 0
* add missing local

The rest LGTM.
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

[1] https://patchwork.ozlabs.org/project/ltp/patch/20210329174135.23890-1-pvorel@suse.cz/

--- testcases/kernel/connectors/pec/cn_pec.sh
+++ testcases/kernel/connectors/pec/cn_pec.sh
@@ -23,7 +23,6 @@ setup()
 {
 	if ! grep -q cn_proc /proc/net/connector; then
 		tst_brk TCONF "Process Event Connector is not supported or kernel is below 2.6.26"
-		exit 0;
 	fi
 
 	tst_res TINFO "Test process events connector"
@@ -32,15 +31,15 @@ setup()
 test()
 {
 	local event=$2
-	pec_listener >lis_$event.log 2>lis_$event.err &
+	local expected_events pid
+
+	ROD pec_listener \>lis_$event.log 2\>lis_$event.err &
 	pid=$!
 	# Wait for pec_listener to start listening
 	tst_sleep 100ms
 
-	# Run with absolute path, so the generator can exec itself
-	generator="$(command -v event_generator)"
-	"$generator" -n $NUM_EVENTS -e $event >gen_$event.log 2>gen_$event.err
-	gen_rc=$?
+	# generator must be in PATH
+	ROD event_generator -n $NUM_EVENTS -e $event \>gen_$event.log 2\>gen_$event.err
 
 	# Sleep until pec_listener has seen and handled all of the generated events
 	tst_sleep 100ms
@@ -48,7 +47,7 @@ test()
 	wait $pid
 	lis_rc=$?
 
-	if [ $gen_rc -ne 0 -o ! -s gen_$event.log ]; then
+	if [ ! -s gen_$event.log ]; then
 		tst_brk TBROK "failed to generate process events"
 	fi
 
diff --git testcases/kernel/connectors/pec/event_generator.c testcases/kernel/connectors/pec/event_generator.c
index f041341f9..f374688ba 100644
--- testcases/kernel/connectors/pec/event_generator.c
+++ testcases/kernel/connectors/pec/event_generator.c
@@ -86,7 +86,7 @@ static void gen_exec(void)
 	fflush(stdout);
 
 	/* Note: This expects the full path to self in exec_argv[0]! */
-	SAFE_EXECV(exec_argv[0], exec_argv);
+	SAFE_EXECVP(exec_argv[0], exec_argv);
 }
 
 /*

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [LTP] [PATCH 2/2] pec: Fix multiple event test
  2021-03-15  9:28 ` [LTP] [PATCH 2/2] pec: Fix multiple event test Joerg Vehlow
@ 2021-03-29 18:08   ` Petr Vorel
  2021-03-29 18:43   ` Petr Vorel
  2021-03-30  8:30   ` Petr Vorel
  2 siblings, 0 replies; 11+ messages in thread
From: Petr Vorel @ 2021-03-29 18:08 UTC (permalink / raw)
  To: ltp

Hi Joerg,

Test is defined as #!/bin/bash which is not acceptable [1], I'm going to change
it as #!/bin/sh. Also note a need to check with checkbashism.

Kind regards,
Petr

[1] https://github.com/linux-test-project/ltp/wiki/Test-Writing-Guidelines#132-shell-coding-style

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [LTP] [PATCH 2/2] pec: Fix multiple event test
  2021-03-15  9:28 ` [LTP] [PATCH 2/2] pec: Fix multiple event test Joerg Vehlow
  2021-03-29 18:08   ` Petr Vorel
@ 2021-03-29 18:43   ` Petr Vorel
  2021-04-15  9:06     ` Joerg Vehlow
  2021-03-30  8:30   ` Petr Vorel
  2 siblings, 1 reply; 11+ messages in thread
From: Petr Vorel @ 2021-03-29 18:43 UTC (permalink / raw)
  To: ltp

Hi Joerg,

> +free_fd()
> +{
> +	# Find a free file handle
> +	local found
> +	for fd in $(seq 200); do
> +    	rco="$(true 2>/dev/null >&${fd}; echo $?)"
> +    	rci="$(true 2>/dev/null <&${fd}; echo $?)"
> +    	[[ "${rco}${rci}" = "11" ]] && found=${fd} && break
[[ .. ]] is a bashism, single brackets are enough.
> +	done
> +	echo $found
> +}

NIT: adding a comment to before function and space separating section with local
helps readability.

# Find a free file handle
free_fd()
{
	local found

	for fd in $(seq 200); do
	...

> +
>  setup()
>  {	
>  	if ! grep -q cn_proc /proc/net/connector; then
> @@ -32,35 +64,75 @@ setup()
>  test()
>  {
>  	local event=$2
> +
> +	tst_res TINFO "Testing $2 event (nevents=$num_events)"
> +
>  	pec_listener >lis_$event.log 2>lis_$event.err &
> -	pid=$!
> +	lis_pid=$!
This change is unnecessary, if you prefer $lis_pid, it should have been in
previous patch where you added $pid (no need to repost, I can fix it before
merge).

>  	# Wait for pec_listener to start listening
>  	tst_sleep 100ms

>  	# Run with absolute path, so the generator can exec itself
>  	generator="$(command -v event_generator)"
> -	"$generator" -n $NUM_EVENTS -e $event >gen_$event.log 2>gen_$event.err
> +	"$generator" -n $num_events -e $event >gen_$event.log 2>gen_$event.err
>  	gen_rc=$?

> -	# Sleep until pec_listener has seen and handled all of the generated events
> -	tst_sleep 100ms
> -	kill -s SIGINT $pid 2> /dev/null
> -	wait $pid
> +	kill -s SIGINT $lis_pid 2> /dev/null
> +	wait $lis_pid
>  	lis_rc=$?

>  	if [ $gen_rc -ne 0 -o ! -s gen_$event.log ]; then
> -		tst_brk TBROK "failed to generate process events"
> +		tst_brk TBROK "failed to generate process events: $(cat gen_$event.err)"
>  	fi

>  	if [ $lis_rc -ne 0 ]; then
>  		tst_brk TBROK "failed to execute the listener: $(cat lis_$event.err)"
>  	fi

> -	expected_events="$(cat gen_$event.log)"
> -	if grep -q "$expected_events" lis_$event.log; then
> -		tst_res TPASS "$event detected by listener"
> +	# The listener writes the same messages as the generator, but it can
> +	# also see more events (e.g. for testing exit, a fork is generated).
> +	# So: The events generated by the generator have to be in the same order
> +	# as the events printed by the listener, but my interleaved with other
> +	# messages. To correctly compare them, we have to open both logs
> +	# and iterate over both of them at the same time, skipping messages
> +	# in the listener log, that are not of interest.
> +	# Because some messages may be multiple times in the listener log,
> +	# we have to open it only once!
> +	# This however does not check, if the listener sees more messages,
> +	# than expected.
> +
> +	fd_act=$(free_fd)
> +	[ -z "$fd_act" ] && tst_brk TBROK "No free filehandle found"
> +	eval "exec ${fd_act}<lis_$event.log"
> +
> +	failed=0
> +	act_nevents=0
Again two missing local.

> +	while read -r exp; do
> +		local found=0
> +		act_nevents=$((act_nevents + 1))
> +		while read -r <&${fd_act} act; do
<& is a bashism. Isn't it using just stdin enough?
		while read -r < $fd_act act; do
> +			if [ "$exp" = "$act" ]; then
> +				found=1
> +				break
> +			fi
> +		done
> +		if [ $found -ne 1 ]; then
> +			failed=1
> +			tst_res TFAIL "Event was not detected by the event listener: $exp"
> +			break
> +		fi
> +	done <gen_$event.log
> +
> +	eval "exec ${fd_act}<&-"
> +
> +	if [ $failed -eq 0 ]; then
> +		if [ $act_nevents -ne $num_events ]; then
> +			tst_res TBROK "Expected $num_events, but $act_nevents generated"
> +		else
> +			tst_res TPASS "All events detected"
> +		fi
>  	else
> -		tst_res TFAIL "$event not detected by listener"
> +		cat lis_$event.log
Why removing tst_res TFAIL?
If "cat lis_$event.log" needed, why not having it in previous commit?
>  	fi

Also whole section would be probably more readable written as:

	if [ $failed -eq 0 ]; then
		tst_res TFAIL "$event not detected by listener"
		cat lis_$event.log
		return
	fi

	if [ $act_nevents -ne $num_events ]; then
		tst_brk TBROK "Expected $num_events, but $act_nevents generated"
	fi

	tst_res TPASS "All events detected"

>  }


All changes suggested for shell:
(FYI in: https://github.com/pevik/ltp/commits/joerg/connectors.v1.fixes)

Kind regards,
Petr

diff --git testcases/kernel/connectors/pec/cn_pec.sh testcases/kernel/connectors/pec/cn_pec.sh
index 8bbfe3a19..e0821a8ef 100755
--- testcases/kernel/connectors/pec/cn_pec.sh
+++ testcases/kernel/connectors/pec/cn_pec.sh
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/bin/sh
 
 # SPDX-License-Identifier: GPL-2.0-or-later
 # Copyright (c) 2008 FUJITSU LIMITED
@@ -39,14 +39,18 @@ parse_args()
 	esac
 }
 
+# Find a free file handle
 free_fd()
 {
-	# Find a free file handle
-	local found
+	local fd found rci rco
+
 	for fd in $(seq 200); do
-	rco="$(true 2>/dev/null >&${fd}; echo $?)"
-	rci="$(true 2>/dev/null <&${fd}; echo $?)"
-	[[ "${rco}${rci}" = "11" ]] && found=${fd} && break
+		rco="$(true 2>/dev/null >&$fd; echo $?)"
+		rci="$(true 2>/dev/null <&$fd; echo $?)"
+		if [ "${rco}${rci}" = "11" ]; then
+			found="$fd"
+			break
+		fi
 	done
 	echo $found
 }
@@ -55,7 +59,6 @@ setup()
 {
 	if ! grep -q cn_proc /proc/net/connector; then
 		tst_brk TCONF "Process Event Connector is not supported or kernel is below 2.6.26"
-		exit 0;
 	fi
 
 	tst_res TINFO "Test process events connector"
@@ -64,24 +67,24 @@ setup()
 test()
 {
 	local event=$2
+	local expected_events lis_rc pid
 
 	tst_res TINFO "Testing $2 event (nevents=$num_events)"
 
-	pec_listener >lis_$event.log 2>lis_$event.err &
-	lis_pid=$!
+	ROD pec_listener \>lis_$event.log 2\>lis_$event.err &
+	pid=$!
 	# Wait for pec_listener to start listening
 	tst_sleep 100ms
 
-	# Run with absolute path, so the generator can exec itself
-	generator="$(command -v event_generator)"
-	"$generator" -n $num_events -e $event >gen_$event.log 2>gen_$event.err
+	# generator must be in PATH
+	ROD event_generator -n $num_events -e $event \>gen_$event.log 2\>gen_$event.err
 	gen_rc=$?
 
 	kill -s SIGINT $lis_pid 2> /dev/null
 	wait $lis_pid
 	lis_rc=$?
 
-	if [ $gen_rc -ne 0 -o ! -s gen_$event.log ]; then
+	if [ ! -s gen_$event.log ]; then
 		tst_brk TBROK "failed to generate process events: $(cat gen_$event.err)"
 	fi
 
@@ -103,14 +106,14 @@ test()
 
 	fd_act=$(free_fd)
 	[ -z "$fd_act" ] && tst_brk TBROK "No free filehandle found"
-	eval "exec ${fd_act}<lis_$event.log"
+	eval "exec ${fd_act} < lis_$event.log"
 
-	failed=0
-	act_nevents=0
+	local failed=0
+	local act_nevents=0
 	while read -r exp; do
 		local found=0
 		act_nevents=$((act_nevents + 1))
-		while read -r <&${fd_act} act; do
+		while read -r < $fd_act act; do
 			if [ "$exp" = "$act" ]; then
 				found=1
 				break
@@ -121,19 +124,21 @@ test()
 			tst_res TFAIL "Event was not detected by the event listener: $exp"
 			break
 		fi
-	done <gen_$event.log
+	done < gen_$event.log
 
-	eval "exec ${fd_act}<&-"
+	eval "exec $fd_act <&-"
 
 	if [ $failed -eq 0 ]; then
-		if [ $act_nevents -ne $num_events ]; then
-			tst_res TBROK "Expected $num_events, but $act_nevents generated"
-		else
-			tst_res TPASS "All events detected"
-		fi
-	else
+		tst_res TFAIL "$event not detected by listener"
 		cat lis_$event.log
+		return
 	fi
+
+	if [ $act_nevents -ne $num_events ]; then
+		tst_brk TBROK "Expected $num_events, but $act_nevents generated"
+	fi
+
+	tst_res TPASS "All events detected"
 }
 
 tst_run

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [LTP] [PATCH 2/2] pec: Fix multiple event test
  2021-03-15  9:28 ` [LTP] [PATCH 2/2] pec: Fix multiple event test Joerg Vehlow
  2021-03-29 18:08   ` Petr Vorel
  2021-03-29 18:43   ` Petr Vorel
@ 2021-03-30  8:30   ` Petr Vorel
  2021-04-01 11:30     ` Petr Vorel
  2 siblings, 1 reply; 11+ messages in thread
From: Petr Vorel @ 2021-03-30  8:30 UTC (permalink / raw)
  To: ltp

Hi Joerg,

I was not able to run this test without failures (using your original test, just
adding missing SAFE_EXECV(), see logs below.

Can you please review my fixes to first commit [1]
and after I merge it send v2 of this patch?

Thanks for working on this!

Kind regards,
Petr

[1] http://lists.linux.it/pipermail/ltp/2021-March/021771.html

cn_pec 1 TINFO: timeout per run is 0h 5m 0s
cn_pec 1 TINFO: Test process events connector
cn_pec 1 TINFO: Testing fork event (nevents=10)
cn_pec 1 TPASS: All events detected
cn_pec 2 TINFO: Testing exec event (nevents=10)
cn_pec 2 TPASS: All events detected
cn_pec 3 TINFO: Testing exit event (nevents=10)
cn_pec 3 TFAIL: Event was not detected by the event listener: exit pid: 20816 exit_code: 0
none err: 0
exit pid: 20813 exit_code: 0 exit_signal: 17
fork parent: 20737, child: 20814
exit pid: 20814 exit_code: 0 exit_signal: 17
fork parent: 20737, child: 20815
exec pid: 20815
fork parent: 20815, child: 20816
exit pid: 20816 exit_code: 0 exit_signal: 17
fork parent: 20815, child: 20817
exit pid: 20817 exit_code: 0 exit_signal: 17
fork parent: 20815, child: 20818
exit pid: 20818 exit_code: 0 exit_signal: 17
fork parent: 20815, child: 20819
exit pid: 20819 exit_code: 0 exit_signal: 17
fork parent: 20815, child: 20820
exit pid: 20820 exit_code: 0 exit_signal: 17
fork parent: 20815, child: 20821
exit pid: 20821 exit_code: 0 exit_signal: 17
fork parent: 20815, child: 20822
exit pid: 20822 exit_code: 0 exit_signal: 17
fork parent: 20815, child: 20823
exit pid: 20823 exit_code: 0 exit_signal: 17
fork parent: 20815, child: 20824
exit pid: 20824 exit_code: 0 exit_signal: 17
fork parent: 20815, child: 20825
exit pid: 20825 exit_code: 0 exit_signal: 17
exit pid: 20815 exit_code: 0 exit_signal: 17
cn_pec 4 TINFO: Testing uid event (nevents=10)
cn_pec 4 TFAIL: Event was not detected by the event listener: uid pid: 20844 euid: 65534
none err: 0
exit pid: 17409 exit_code: 0 exit_signal: 17
exit pid: 20842 exit_code: 0 exit_signal: 17
fork parent: 20737, child: 20843
exit pid: 20843 exit_code: 0 exit_signal: 17
fork parent: 20737, child: 20844
exec pid: 20844
uid pid: 20844 euid: 65534 ruid: 65534
exit pid: 20844 exit_code: 0 exit_signal: 17
cn_pec 5 TINFO: Testing gid event (nevents=10)
cn_pec 5 TFAIL: Event was not detected by the event listener: gid pid: 20863 egid: 65533
none err: 0
exit pid: 20861 exit_code: 0 exit_signal: 17
fork parent: 20737, child: 20862
exit pid: 20862 exit_code: 0 exit_signal: 17
fork parent: 20737, child: 20863
exec pid: 20863
gid pid: 20863 egid: 65533 rgid: 65533
exit pid: 20863 exit_code: 0 exit_signal: 17
cn_pec 6 TINFO: AppArmor enabled, this may affect test results
cn_pec 6 TINFO: it can be disabled with TST_DISABLE_APPARMOR=1 (requires super/root)
cn_pec 6 TINFO: loaded AppArmor profiles: none

Summary:
passed   2
failed   3
skipped  0
warnings 0


Kind regards,
Petr

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [LTP] [PATCH 2/2] pec: Fix multiple event test
  2021-03-30  8:30   ` Petr Vorel
@ 2021-04-01 11:30     ` Petr Vorel
  0 siblings, 0 replies; 11+ messages in thread
From: Petr Vorel @ 2021-04-01 11:30 UTC (permalink / raw)
  To: ltp

Hi Joerg,


> I was not able to run this test without failures (using your original test, just
> adding missing SAFE_EXECV(), see logs below.

Merged this one. Thanks for your work.

Could you please send v2 for the second patch?

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [LTP] [PATCH 2/2] pec: Fix multiple event test
  2021-03-29 18:43   ` Petr Vorel
@ 2021-04-15  9:06     ` Joerg Vehlow
  0 siblings, 0 replies; 11+ messages in thread
From: Joerg Vehlow @ 2021-04-15  9:06 UTC (permalink / raw)
  To: ltp

Hi Petr,

I will send a v2, but want to comment on some of your remarks first:

On 3/29/2021 8:43 PM, Petr Vorel wrote:
>> +	while read -r exp; do
>> +		local found=0
>> +		act_nevents=$((act_nevents + 1))
>> +		while read -r <&${fd_act} act; do
> <& is a bashism. Isn't it using just stdin enough?
Actually no, and I don't even think this is a bashism. I guess it is a 
bug in the checkbashisms script.
[n]<&m means copy file descriptor m to n when executing the command. See 
[1].
My theory of a bug is even more supported by the fact, that 
checkbashisms suggests to replace
"<&$var" with ">word 2>&1". so "<&$var" obviously triggered the 
"&>word"-rule.

This is required here, because $fd_act is a file handle to the actual 
received events, that is supposed
to be iterated only once, while the expected events are iterated over in 
the outer loop.
This construct allows "actual" to contain more events, than expected. 
But all expected events must be
in the correct order in actual.

In v2 I reordered the code a bit (while read....; do ....; done <&$fd). 
This still triggers checkbashims, but
it this works@least in bash, ash, dash, and busybox sh.

> 		while read -r < $fd_act act; do
>> +			if [ "$exp" = "$act" ]; then
>> +				found=1
>> +				break
>> +			fi
>> +		done
>> +		if [ $found -ne 1 ]; then
>> +			failed=1
>> +			tst_res TFAIL "Event was not detected by the event listener: $exp"
>> +			break
>> +		fi
>> +	done <gen_$event.log
>> +
>> +	eval "exec ${fd_act}<&-"
>> +
>> +	if [ $failed -eq 0 ]; then
>> +		if [ $act_nevents -ne $num_events ]; then
>> +			tst_res TBROK "Expected $num_events, but $act_nevents generated"
>> +		else
>> +			tst_res TPASS "All events detected"
>> +		fi
>>   	else
>> -		tst_res TFAIL "$event not detected by listener"
>> +		cat lis_$event.log
> Why removing tst_res TFAIL?
> If "cat lis_$event.log" needed, why not having it in previous commit?
I moved this up into the loop, to be able to output the expected event, 
that was not detected.
The output of the actual events is for debugging. It is not strictly 
required,
but could simplify identifying why an event is missing. Feel free to 
remove this in v2 ;)

I hope I fixed everything else.

J?rg

[1] 
https://pubs.opengroup.org/onlinepubs/009604499/utilities/xcu_chap02.html#tag_02_07_05

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-04-15  9:06 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-15  9:28 [LTP] [PATCH 1/2] pec: Convert to the new API Joerg Vehlow
2021-03-15  9:28 ` [LTP] [PATCH 2/2] pec: Fix multiple event test Joerg Vehlow
2021-03-29 18:08   ` Petr Vorel
2021-03-29 18:43   ` Petr Vorel
2021-04-15  9:06     ` Joerg Vehlow
2021-03-30  8:30   ` Petr Vorel
2021-04-01 11:30     ` Petr Vorel
2021-03-17  9:34 ` [LTP] [PATCH 1/2] pec: Convert to the new API Petr Vorel
2021-03-17 10:05   ` Joerg Vehlow
2021-03-17 11:47     ` Petr Vorel
2021-03-29 17:56 ` Petr Vorel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.