[LTP] [PATCH 2/2] pec: Fix multiple event test

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
	...

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