[LTP] [PATCH v9 0/4] Improve ioctl02.c

[LTP] [PATCH v9 0/4] Improve ioctl02.c

[Marius Kittler]

I fixed the closing of parentfd but only in the last patch
because in the first patch the cleanup function already takes care
of it sufficiently.

I also used TCIOFLUSH as suggested in the review.

I also fixed the spelling of one of the macros.

Marius Kittler (4):
  Refactor ioctl02.c to use the new test API
  Make checks for termio flags more strict
  Remove disabled code in ioctl02.c
  Extend ioctl02 to test termio and termios

 testcases/kernel/syscalls/ioctl/ioctl02.c | 552 +++++++---------------
 1 file changed, 172 insertions(+), 380 deletions(-)

-- 
2.42.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v9 1/4] Refactor ioctl02.c to use the new test API

[Marius Kittler]

* Use checkpoint API instead of signals
* Use SAFE_…-macros instead of manual error handling
* See https://github.com/linux-test-project/ltp/issues/637
  for related discussion.

Signed-off-by: Marius Kittler <mkittler@suse.de>
---
 testcases/kernel/syscalls/ioctl/ioctl02.c | 414 +++++-----------------
 1 file changed, 88 insertions(+), 326 deletions(-)

diff --git a/testcases/kernel/syscalls/ioctl/ioctl02.c b/testcases/kernel/syscalls/ioctl/ioctl02.c
index b4d4a3594..c2ad048fe 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl02.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl02.c
@@ -1,228 +1,86 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- *   Copyright (c) International Business Machines  Corp., 2001
- *   Copyright (c) 2020 Petr Vorel <pvorel@suse.cz>
- *
- *   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
+ * Copyright (c) International Business Machines  Corp., 2001
+ * Copyright (c) 2020 Petr Vorel <pvorel@suse.cz>
+ * Copyright (c) 2023 Marius Kittler <mkittler@suse.de>
  */
 
-/*
- * NAME
- *	ioctl02.c
- *
- * DESCRIPTION
- *	Testcase to test the TCGETA, and TCSETA ioctl implementations for
- *	the tty driver
- *
- * ALGORITHM
- *	In this test, the parent and child open the parentty and the childtty
- *	respectively.  After opening the childtty the child flushes the stream
- *	and sends a SIGUSR1 to the parent (thereby asking it to continue its
- *	testing). The parent, which was waiting for this signal to arrive, now
- *	starts the testing. It issues a TCGETA ioctl to get all the tty
- *	parameters. It then changes them to known values by issuing a TCSETA
- *	ioctl.  Then the parent issues a TCGETA ioctl again and compares the
- *	received values with what it had set earlier. The test fails if TCGETA
- *	or TCSETA fails, or if the received values don't match those that were
- *	set. The parent does all the testing, the requirement of the child
- *	process is to moniter the testing done by the parent, and hence the
- *	child just waits for the parent.
+/*\
+ * [Description]
  *
- * USAGE:  <for command-line>
- *  ioctl02 -D /dev/tty[0-9] [-c n] [-f] [-i n] [-I x] [-P x] [-t]
- *     where,  -c n : Run n copies concurrently.
- *             -f   : Turn off functionality Testing.
- *             -i n : Execute test n times.
- *             -I x : Execute test for x seconds.
- *             -P x : Pause for x seconds between iterations.
- *             -t   : Turn on syscall timing.
+ * Testcase to test the TCGETA, and TCSETA ioctl implementations for
+ * the tty driver
  *
- * HISTORY
- *	07/2001 Ported by Wayne Boyer
- *
- * RESTRICTIONS
- *	test must be run with the -D option
- *	test may have to be run as root depending on the tty permissions
+ * In this test, the parent and child open the parentty and the childtty
+ * respectively.  After opening the childtty the child flushes the stream
+ * and wakes the parent (thereby asking it to continue its testing). The
+ * parent, then starts the testing. It issues a TCGETA ioctl to get all
+ * the tty parameters. It then changes them to known values by issuing a
+ * TCSETA ioctl. Then the parent issues a TCGETA ioctl again and compares
+ * the received values with what it had set earlier. The test fails if
+ * TCGETA or TCSETA fails, or if the received values don't match those
+ * that were set. The parent does all the testing, the requirement of the
+ * child process is to moniter the testing done by the parent, and hence
+ * the child just waits for the parent.
  */
 
 #include <stdio.h>
+#include <stdlib.h>
 #include <fcntl.h>
-#include <signal.h>
 #include <errno.h>
 #include <sys/wait.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <termios.h>
-#include "test.h"
-#include "safe_macros.h"
-#include "lapi/ioctl.h"
 
-#define	CNUL	0
+#include "lapi/ioctl.h"
 
-char *TCID = "ioctl02";
-int TST_TOTAL = 1;
+#include "tst_checkpoint.h"
+#include "tst_test.h"
+#include "tst_safe_macros.h"
 
 static struct termio termio, save_io;
 
 static char *parenttty, *childtty;
-static int parentfd, childfd;
+static int parentfd = -1;
 static int parentpid, childpid;
-static volatile int sigterm, sigusr1, sigusr2;
-static int closed = 1;
-
-static int do_child_setup(void);
-static int do_parent_setup(void);
-static int run_ptest(void);
-static int run_ctest(void);
-static int chk_tty_parms();
+
+static void do_child(void);
+static void run_ptest(void);
+static void chk_tty_parms(void);
 static void setup(void);
 static void cleanup(void);
-static void help(void);
-static void do_child(void);
-void do_child_uclinux(void);
-static void sigterm_handler(void);
-
-static int Devflag;
-static char *devname;
 
-static option_t options[] = {
-	{"D:", &Devflag, &devname},
-	{NULL, NULL, NULL}
-};
+static char *device;
 
-int main(int ac, char **av)
+static void verify_ioctl(void)
 {
-	int lc;
-	int rval;
-
-	tst_parse_opts(ac, av, options, &help);
-
-#ifdef UCLINUX
-	maybe_run_child(&do_child_uclinux, "dS", &parentpid, &childtty);
-#endif
-
-	if (!Devflag)
-		tst_brkm(TBROK, NULL, "You must specify a tty device with "
-			 "the -D option.");
-
-	tst_require_root();
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-
-		tst_count = 0;
-
-		parenttty = devname;
-		childtty = devname;
-
-		parentpid = getpid();
-
-		childpid = FORK_OR_VFORK();
-		if (childpid < 0)
-			tst_brkm(TBROK, cleanup, "fork failed");
-
-		if (childpid == 0) {	/* child */
-#ifdef UCLINUX
-			if (self_exec(av[0], "dS", parentpid, childtty) < 0)
-				tst_brkm(TBROK, cleanup, "self_exec failed");
-#else
-			do_child();
-#endif
-		}
-
-		while (!sigusr1)
-			sleep(1);
-
-		sigusr1 = 0;
-
-		parentfd = do_parent_setup();
-		if (parentfd < 0) {
-			kill(childpid, SIGTERM);
-			waitpid(childpid, NULL, 0);
-			cleanup();
-		}
-
-		/* run the parent test */
-		rval = run_ptest();
-		if (rval == -1) {
-			/*
-			 * Parent cannot set/get ioctl parameters.
-			 * SIGTERM the child and cleanup.
-			 */
-			kill(childpid, SIGTERM);
-			waitpid(childpid, NULL, 0);
-			cleanup();
-		}
-
-		if (rval != 0)
-			tst_resm(TFAIL, "TCGETA/TCSETA tests FAILED with "
-				 "%d %s", rval, rval > 1 ? "errors" : "error");
-		else
-			tst_resm(TPASS, "TCGETA/TCSETA tests SUCCEEDED");
-
-		/* FIXME: check return codes. */
-		(void)kill(childpid, SIGTERM);
-		(void)waitpid(childpid, NULL, 0);
-
-		/*
-		 * Clean up things from the parent by restoring the
-		 * tty device information that was saved in setup()
-		 * and closing the tty file descriptor.
-		 */
-		if (ioctl(parentfd, TCSETA, &save_io) == -1)
-			tst_resm(TINFO, "ioctl restore failed in main");
-		SAFE_CLOSE(cleanup, parentfd);
-
-		closed = 1;
+	parenttty = device;
+	childtty = device;
+
+	parentpid = getpid();
+	childpid = SAFE_FORK();
+	if (!childpid) {
+		do_child();
+		exit(EXIT_SUCCESS);
 	}
-	cleanup();
 
-	tst_exit();
-}
+	TST_CHECKPOINT_WAIT(0);
 
-static void do_child(void)
-{
-	childfd = do_child_setup();
-	if (childfd < 0)
-		_exit(1);
-	run_ctest();
-	_exit(0);
-}
+	parentfd = SAFE_OPEN(parenttty, O_RDWR, 0777);
+	SAFE_IOCTL(parentfd, TCFLSH, TCIOFLUSH);
 
-void do_child_uclinux(void)
-{
-	struct sigaction act;
+	run_ptest();
 
-	/* Set up the signal handlers again */
-	act.sa_handler = (void *)sigterm_handler;
-	act.sa_flags = 0;
-	sigemptyset(&act.sa_mask);
-	(void)sigaction(SIGTERM, &act, 0);
-
-	/* Run the normal child */
-	do_child();
+	TST_CHECKPOINT_WAKE(0);
 }
 
 /*
  * run_ptest() - setup the various termio structure values and issue
  *		 the TCSETA ioctl call with the TEST macro.
  */
-static int run_ptest(void)
+static void run_ptest(void)
 {
-	int i, rval;
-
 	/* Use "old" line discipline */
 	termio.c_line = 0;
 
@@ -230,7 +88,7 @@ static int run_ptest(void)
 	termio.c_cflag = B50 | CS7 | CREAD | PARENB | PARODD | CLOCAL;
 
 	/* Set control chars. */
-	for (i = 0; i < NCC; i++) {
+	for (int i = 0; i < NCC; i++) {
 		if (i == VEOL2)
 			continue;
 		termio.c_cc[i] = CSTART;
@@ -248,47 +106,19 @@ static int run_ptest(void)
 	/* Set output modes. */
 	termio.c_oflag = OPOST | OLCUC | ONLCR | ONOCR;
 
-	TEST(ioctl(parentfd, TCSETA, &termio));
-
-	if (TEST_RETURN < 0) {
-		tst_resm(TFAIL, "ioctl TCSETA failed : "
-			 "errno = %d", TEST_ERRNO);
-		return -1;
-	}
+	SAFE_IOCTL(parentfd, TCSETA, &termio);
 
 	/* Get termio and see if all parameters actually got set */
-	rval = ioctl(parentfd, TCGETA, &termio);
-	if (rval < 0) {
-		tst_resm(TFAIL, "ioctl TCGETA failed.  Ending test.");
-		return -1;
-	}
-
-	return chk_tty_parms();
+	SAFE_IOCTL(parentfd, TCGETA, &termio);
+	chk_tty_parms();
 }
 
-static int run_ctest(void)
-{
-	/*
-	 * Wait till the parent has finished testing.
-	 */
-	while (!sigterm)
-		sleep(1);
-
-	sigterm = 0;
-
-	tst_resm(TINFO, "child: Got SIGTERM from parent.");
-
-	if (close(childfd) == -1)
-		tst_resm(TINFO, "close() in run_ctest() failed");
-	return 0;
-}
-
-static int chk_tty_parms(void)
+static void chk_tty_parms(void)
 {
 	int i, flag = 0;
 
 	if (termio.c_line != 0) {
-		tst_resm(TINFO, "line discipline has incorrect value %o",
+		tst_res(TFAIL, "line discipline has incorrect value %o",
 			 termio.c_line);
 		flag++;
 	}
@@ -301,7 +131,7 @@ static int chk_tty_parms(void)
 	 */
 #if 0
 	if (termio.c_cflag != (B50 | CS7 | CREAD | PARENB | PARODD | CLOCAL)) {
-		tst_resm(TINFO, "cflag has incorrect value. %o",
+		tst_res(TFAIL, "cflag has incorrect value. %o",
 			 termio.c_cflag);
 		flag++;
 	}
@@ -309,19 +139,18 @@ static int chk_tty_parms(void)
 
 	for (i = 0; i < NCC; i++) {
 		if (i == VEOL2) {
-			if (termio.c_cc[VEOL2] == CNUL) {
+			if (!termio.c_cc[i]) {
 				continue;
 			} else {
-				tst_resm(TINFO, "control char %d has "
-					 "incorrect value %d %d", i,
-					 termio.c_cc[i], CNUL);
+				tst_res(TFAIL, "control char %d has "
+					 "incorrect value %d", i, termio.c_cc[i]);
 				flag++;
 				continue;
 			}
 		}
 
 		if (termio.c_cc[i] != CSTART) {
-			tst_resm(TINFO, "control char %d has incorrect "
+			tst_res(TFAIL, "control char %d has incorrect "
 				 "value %d.", i, termio.c_cc[i]);
 			flag++;
 		}
@@ -330,7 +159,7 @@ static int chk_tty_parms(void)
 	if (!
 	    (termio.c_lflag
 	     && (ISIG | ICANON | XCASE | ECHO | ECHOE | NOFLSH))) {
-		tst_resm(TINFO, "lflag has incorrect value. %o",
+		tst_res(TFAIL, "lflag has incorrect value. %o",
 			 termio.c_lflag);
 		flag++;
 	}
@@ -339,130 +168,63 @@ static int chk_tty_parms(void)
 	    (termio.c_iflag
 	     && (BRKINT | IGNPAR | INPCK | ISTRIP | ICRNL | IUCLC | IXON | IXANY
 		 | IXOFF))) {
-		tst_resm(TINFO, "iflag has incorrect value. %o",
+		tst_res(TFAIL, "iflag has incorrect value. %o",
 			 termio.c_iflag);
 		flag++;
 	}
 
 	if (!(termio.c_oflag && (OPOST | OLCUC | ONLCR | ONOCR))) {
-		tst_resm(TINFO, "oflag has incorrect value. %o",
+		tst_res(TFAIL, "oflag has incorrect value. %o",
 			 termio.c_oflag);
 		flag++;
 	}
 
 	if (!flag)
-		tst_resm(TINFO, "termio values are set as expected");
-
-	return flag;
+		tst_res(TPASS, "TCGETA/TCSETA tests");
 }
 
-static int do_parent_setup(void)
-{
-	int pfd;
-
-	pfd = SAFE_OPEN(cleanup, parenttty, O_RDWR, 0777);
-
-	/* unset the closed flag */
-	closed = 0;
-
-	/* flush tty queues to remove old output */
-	SAFE_IOCTL(cleanup, pfd, TCFLSH, 2);
-	return pfd;
-}
-
-static int do_child_setup(void)
+static void do_child(void)
 {
-	int cfd;
-
-	cfd = open(childtty, O_RDWR, 0777);
-	if (cfd < 0) {
-		tst_resm(TINFO, "Could not open %s in do_child_setup(), errno "
-			 "= %d", childtty, errno);
-		/* signal the parent so we don't hang the test */
-		kill(parentpid, SIGUSR1);
-		return -1;
-	}
+	int cfd = SAFE_OPEN(childtty, O_RDWR, 0777);
 
-	/* flush tty queues to remove old output */
-	if (ioctl(cfd, TCFLSH, 2) < 0) {
-		tst_resm(TINFO, "ioctl TCFLSH failed. : errno = %d", errno);
-		/* signal the parent so we don't hang the test */
-		kill(parentpid, SIGUSR1);
-		return -1;
-	}
+	SAFE_IOCTL(cfd, TCFLSH, TCIOFLUSH);
 
 	/* tell the parent that we're done */
-	kill(parentpid, SIGUSR1);
+	TST_CHECKPOINT_WAKE(0);
 
-	return cfd;
-}
-
-/*
- * Define the signals handlers here.
- */
-static void sigterm_handler(void)
-{
-	sigterm = 1;
-}
-
-static void sigusr1_handler(void)
-{
-	sigusr1 = 1;
-}
-
-static void sigusr2_handler(void)
-{
-	sigusr2 = 1;
-}
-
-static void help(void)
-{
-	printf("  -D <tty device> : for example, /dev/tty[0-9]\n");
+	TST_CHECKPOINT_WAIT(0);
+	tst_res(TINFO, "child: parent has finished testing");
+	SAFE_CLOSE(cfd);
 }
 
 static void setup(void)
 {
-	int fd;
-	struct sigaction act;
-
-	/* XXX: TERRNO required all over the place */
-	fd = SAFE_OPEN(NULL, devname, O_RDWR, 0777);
-
-	/* Save the current device information - to be restored in cleanup() */
-	SAFE_IOCTL(cleanup, fd, TCGETA, &save_io);
-
-	/* Close the device */
-	SAFE_CLOSE(cleanup, fd);
-
-	/* Set up the signal handlers */
-	act.sa_handler = (void *)sigterm_handler;
-	act.sa_flags = 0;
-	sigemptyset(&act.sa_mask);
-	(void)sigaction(SIGTERM, &act, 0);
+	if (!device)
+		tst_brk(TBROK, "You must specify a tty device with -D option");
 
-	act.sa_handler = (void *)sigusr1_handler;
-	act.sa_flags = 0;
-	(void)sigaction(SIGUSR1, &act, 0);
+	int fd = SAFE_OPEN(device, O_RDWR, 0777);
 
-	act.sa_handler = (void *)sigusr2_handler;
-	act.sa_flags = 0;
-	(void)sigaction(SIGUSR2, &act, 0);
-
-	act.sa_handler = SIG_IGN;
-	act.sa_flags = 0;
-	(void)sigaction(SIGTTOU, &act, 0);
-
-	sigterm = sigusr1 = sigusr2 = 0;
-
-	TEST_PAUSE;
+	SAFE_IOCTL(fd, TCGETA, &save_io);
+	SAFE_CLOSE(fd);
 }
 
 static void cleanup(void)
 {
-	if (!closed) {
-		if (ioctl(parentfd, TCSETA, &save_io) == -1)
-			tst_resm(TINFO, "ioctl restore failed in cleanup()");
-		if (close(parentfd) == -1)
-			tst_resm(TINFO, "close() failed in cleanup()");
+	if (parentfd >= 0) {
+		SAFE_IOCTL(parentfd, TCSETA, &save_io);
+		SAFE_CLOSE(parentfd);
 	}
 }
+
+static struct tst_test test = {
+	.needs_root = 1,
+	.needs_checkpoints = 1,
+	.forks_child = 1,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = verify_ioctl,
+	.options = (struct tst_option[]) {
+		{"D:", &device, "Tty device. For example, /dev/tty[0-9]"},
+		{}
+	}
+};
\ No newline at end of file
-- 
2.42.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v9 1/4] Refactor ioctl02.c to use the new test API

[Petr Vorel]

Hi Marius,

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

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v9 2/4] Make checks for termio flags more strict

[Marius Kittler]

The checks for termio flags can actually use a `!=` check to
also fail when any unexpected flags are present.

Signed-off-by: Marius Kittler <mkittler@suse.de>
---
 testcases/kernel/syscalls/ioctl/ioctl02.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/testcases/kernel/syscalls/ioctl/ioctl02.c b/testcases/kernel/syscalls/ioctl/ioctl02.c
index c2ad048fe..4a502a724 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl02.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl02.c
@@ -156,24 +156,21 @@ static void chk_tty_parms(void)
 		}
 	}
 
-	if (!
-	    (termio.c_lflag
-	     && (ISIG | ICANON | XCASE | ECHO | ECHOE | NOFLSH))) {
+	if (termio.c_lflag != (ISIG | ICANON | XCASE | ECHO | ECHOE
+		 | NOFLSH)) {
 		tst_res(TFAIL, "lflag has incorrect value. %o",
 			 termio.c_lflag);
 		flag++;
 	}
 
-	if (!
-	    (termio.c_iflag
-	     && (BRKINT | IGNPAR | INPCK | ISTRIP | ICRNL | IUCLC | IXON | IXANY
-		 | IXOFF))) {
+	if (termio.c_iflag != (BRKINT | IGNPAR | INPCK | ISTRIP
+		 | ICRNL | IUCLC | IXON | IXANY | IXOFF)) {
 		tst_res(TFAIL, "iflag has incorrect value. %o",
 			 termio.c_iflag);
 		flag++;
 	}
 
-	if (!(termio.c_oflag && (OPOST | OLCUC | ONLCR | ONOCR))) {
+	if (termio.c_oflag != (OPOST | OLCUC | ONLCR | ONOCR)) {
 		tst_res(TFAIL, "oflag has incorrect value. %o",
 			 termio.c_oflag);
 		flag++;
-- 
2.42.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v9 2/4] Make checks for termio flags more strict

[Petr Vorel]

Hi Marius,

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

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v9 3/4] Remove disabled code in ioctl02.c

[Marius Kittler]

Signed-off-by: Marius Kittler <mkittler@suse.de>
---
 testcases/kernel/syscalls/ioctl/ioctl02.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/testcases/kernel/syscalls/ioctl/ioctl02.c b/testcases/kernel/syscalls/ioctl/ioctl02.c
index 4a502a724..8c6924386 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl02.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl02.c
@@ -122,20 +122,6 @@ static void chk_tty_parms(void)
 			 termio.c_line);
 		flag++;
 	}
-	/*
-	 * The following Code Sniffet is disabled to check the value of c_cflag
-	 * as it seems that due to some changes from 2.6.24 onwards, this
-	 * setting is not done properly for either of (B50|CS7|CREAD|PARENB|
-	 * PARODD|CLOCAL|(CREAD|HUPCL|CLOCAL).
-	 * However, it has been observed that other flags are properly set.
-	 */
-#if 0
-	if (termio.c_cflag != (B50 | CS7 | CREAD | PARENB | PARODD | CLOCAL)) {
-		tst_res(TFAIL, "cflag has incorrect value. %o",
-			 termio.c_cflag);
-		flag++;
-	}
-#endif
 
 	for (i = 0; i < NCC; i++) {
 		if (i == VEOL2) {
-- 
2.42.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v9 3/4] Remove disabled code in ioctl02.c

[Petr Vorel]

Hi Marius,

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

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v9 3/4] Remove disabled code in ioctl02.c

[Petr Vorel]

Hi all,

thanks, merged these three.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v9 4/4] Extend ioctl02 to test termio and termios

[Marius Kittler]

Signed-off-by: Marius Kittler <mkittler@suse.de>
---
 testcases/kernel/syscalls/ioctl/ioctl02.c | 191 ++++++++++++++--------
 1 file changed, 119 insertions(+), 72 deletions(-)

diff --git a/testcases/kernel/syscalls/ioctl/ioctl02.c b/testcases/kernel/syscalls/ioctl/ioctl02.c
index 8c6924386..febe7cdd8 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl02.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl02.c
@@ -8,26 +8,28 @@
 /*\
  * [Description]
  *
- * Testcase to test the TCGETA, and TCSETA ioctl implementations for
- * the tty driver
+ * Testcase to test the TCGETA/TCGETS, and TCSETA/TCSETS ioctl
+ * implementations for the tty driver
  *
  * In this test, the parent and child open the parentty and the childtty
  * respectively.  After opening the childtty the child flushes the stream
  * and wakes the parent (thereby asking it to continue its testing). The
- * parent, then starts the testing. It issues a TCGETA ioctl to get all
- * the tty parameters. It then changes them to known values by issuing a
- * TCSETA ioctl. Then the parent issues a TCGETA ioctl again and compares
- * the received values with what it had set earlier. The test fails if
- * TCGETA or TCSETA fails, or if the received values don't match those
- * that were set. The parent does all the testing, the requirement of the
- * child process is to moniter the testing done by the parent, and hence
- * the child just waits for the parent.
+ * parent, then starts the testing. It issues a TCGETA/TCGETS ioctl to
+ * get all the tty parameters. It then changes them to known values by
+ * issuing a TCSETA/TCSETS ioctl. Then the parent issues a TCSETA/TCGETS
+ * ioctl again and compares the received values with what it had set
+ * earlier. The test fails if TCGETA/TCGETS or TCSETA/TCSETS fails, or if
+ * the received values don't match those that were set. The parent does
+ * all the testing, the requirement of the child process is to moniter
+ * the testing done by the parent, and hence the child just waits for the
+ * parent.
  */
 
 #include <stdio.h>
 #include <stdlib.h>
 #include <fcntl.h>
 #include <errno.h>
+#include <sys/ioctl.h>
 #include <sys/wait.h>
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -39,22 +41,51 @@
 #include "tst_test.h"
 #include "tst_safe_macros.h"
 
-static struct termio termio, save_io;
+static struct termio termio, termio_exp;
+static struct termios termios, termios_exp, termios_bak;
 
 static char *parenttty, *childtty;
 static int parentfd = -1;
 static int parentpid, childpid;
 
 static void do_child(void);
+static void prepare_termio(void);
 static void run_ptest(void);
-static void chk_tty_parms(void);
+static void chk_tty_parms_termio(void);
+static void chk_tty_parms_termios(void);
 static void setup(void);
 static void cleanup(void);
 
 static char *device;
 
+static struct variant {
+	const char *name;
+	void *termio, *termio_exp, *termio_bak;
+	void (*check)(void);
+	int tcget, tcset;
+} variants[] = {
+	{
+		.name = "termio",
+		.termio = &termio,
+		.termio_exp = &termio_exp,
+		.check = &chk_tty_parms_termio,
+		.tcget = TCGETA,
+		.tcset = TCSETA,
+	},
+	{
+		.name = "termios",
+		.termio = &termios,
+		.termio_exp = &termios_exp,
+		.check = &chk_tty_parms_termios,
+		.tcget = TCGETS,
+		.tcset = TCSETS,
+	},
+};
+
 static void verify_ioctl(void)
 {
+	tst_res(TINFO, "Testing %s variant", variants[tst_variant].name);
+
 	parenttty = device;
 	childtty = device;
 
@@ -73,99 +104,112 @@ static void verify_ioctl(void)
 	run_ptest();
 
 	TST_CHECKPOINT_WAKE(0);
+
+	if (tst_variant < sizeof(variants) - 1)
+		SAFE_CLOSE(parentfd);
 }
 
-/*
- * run_ptest() - setup the various termio structure values and issue
- *		 the TCSETA ioctl call with the TEST macro.
- */
-static void run_ptest(void)
+static void prepare_termio(void)
 {
 	/* Use "old" line discipline */
-	termio.c_line = 0;
+	termios_exp.c_line = termio_exp.c_line = 0;
 
 	/* Set control modes */
-	termio.c_cflag = B50 | CS7 | CREAD | PARENB | PARODD | CLOCAL;
+	termios_exp.c_cflag = termio_exp.c_cflag = B50 | CS7 | CREAD | PARENB | PARODD | CLOCAL;
 
 	/* Set control chars. */
 	for (int i = 0; i < NCC; i++) {
 		if (i == VEOL2)
 			continue;
-		termio.c_cc[i] = CSTART;
+		termios_exp.c_cc[i] = termio_exp.c_cc[i] = CSTART;
 	}
 
 	/* Set local modes. */
-	termio.c_lflag =
+	termios_exp.c_lflag = termio_exp.c_lflag =
 	    ((unsigned short)(ISIG | ICANON | XCASE | ECHO | ECHOE | NOFLSH));
 
 	/* Set input modes. */
-	termio.c_iflag =
+	termios_exp.c_iflag = termio_exp.c_iflag =
 	    BRKINT | IGNPAR | INPCK | ISTRIP | ICRNL | IUCLC | IXON | IXANY |
 	    IXOFF;
 
 	/* Set output modes. */
-	termio.c_oflag = OPOST | OLCUC | ONLCR | ONOCR;
-
-	SAFE_IOCTL(parentfd, TCSETA, &termio);
+	termios_exp.c_oflag = termio_exp.c_oflag = OPOST | OLCUC | ONLCR | ONOCR;
 
-	/* Get termio and see if all parameters actually got set */
-	SAFE_IOCTL(parentfd, TCGETA, &termio);
-	chk_tty_parms();
+	/* Init termio/termios structures used to check if all params got set */
+	memset(&termio, 0, sizeof(termio));
+	memset(&termios, 0, sizeof(termios));
 }
 
-static void chk_tty_parms(void)
+/*
+ * run_ptest() - setup the various termio/termios structure values and issue
+ * the TCSETA/TCSETS ioctl call with the TEST macro.
+ */
+static void run_ptest(void)
 {
-	int i, flag = 0;
+	struct variant *v = &variants[tst_variant];
 
-	if (termio.c_line != 0) {
-		tst_res(TFAIL, "line discipline has incorrect value %o",
-			 termio.c_line);
-		flag++;
-	}
+	SAFE_IOCTL(parentfd, v->tcset, v->termio_exp);
 
-	for (i = 0; i < NCC; i++) {
-		if (i == VEOL2) {
-			if (!termio.c_cc[i]) {
-				continue;
-			} else {
-				tst_res(TFAIL, "control char %d has "
-					 "incorrect value %d", i, termio.c_cc[i]);
-				flag++;
-				continue;
-			}
-		}
-
-		if (termio.c_cc[i] != CSTART) {
-			tst_res(TFAIL, "control char %d has incorrect "
-				 "value %d.", i, termio.c_cc[i]);
-			flag++;
-		}
-	}
-
-	if (termio.c_lflag != (ISIG | ICANON | XCASE | ECHO | ECHOE
-		 | NOFLSH)) {
-		tst_res(TFAIL, "lflag has incorrect value. %o",
-			 termio.c_lflag);
-		flag++;
-	}
+	/* Get termio and see if all parameters actually got set */
+	SAFE_IOCTL(parentfd, v->tcget, v->termio);
+	v->check();
+}
 
-	if (termio.c_iflag != (BRKINT | IGNPAR | INPCK | ISTRIP
-		 | ICRNL | IUCLC | IXON | IXANY | IXOFF)) {
-		tst_res(TFAIL, "iflag has incorrect value. %o",
-			 termio.c_iflag);
-		flag++;
+#define CMP_ATTR(tcexp, tcval, attr) \
+	do { \
+		if ((tcval).attr != (tcexp).attr) { \
+			tst_res(TINFO, #attr " has incorrect value %o", \
+				(tcval).attr); \
+			flag++; \
+		} \
+	} while (0)
+
+#define CHECK_CONTROL_CHARS(tcval) \
+	for (i = 0; i < NCC; i++) { \
+		if (i == VEOL2) { \
+			if (!(tcval).c_cc[i]) { \
+				continue; \
+			} else { \
+				tst_res(TFAIL, "control char %d has " \
+					 "incorrect value %d", i, (tcval).c_cc[i]); \
+				flag++; \
+				continue; \
+			} \
+		} \
+		if ((tcval).c_cc[i] != CSTART) { \
+			tst_res(TFAIL, "control char %d has incorrect " \
+				 "value %d.", i, (tcval).c_cc[i]); \
+			flag++; \
+		} \
 	}
 
-	if (termio.c_oflag != (OPOST | OLCUC | ONLCR | ONOCR)) {
-		tst_res(TFAIL, "oflag has incorrect value. %o",
-			 termio.c_oflag);
-		flag++;
-	}
+static void chk_tty_parms_termio(void)
+{
+	int i, flag = 0;
 
+	CMP_ATTR(termio_exp, termio, c_line);
+	CHECK_CONTROL_CHARS(termio);
+	CMP_ATTR(termio_exp, termio, c_lflag);
+	CMP_ATTR(termio_exp, termio, c_iflag);
+	CMP_ATTR(termio_exp, termio, c_oflag);
 	if (!flag)
 		tst_res(TPASS, "TCGETA/TCSETA tests");
 }
 
+static void chk_tty_parms_termios(void)
+{
+	int i, flag = 0;
+
+	CMP_ATTR(termios_exp, termios, c_line);
+	CHECK_CONTROL_CHARS(termios);
+	CMP_ATTR(termios_exp, termios, c_lflag);
+	CMP_ATTR(termios_exp, termios, c_iflag);
+	CMP_ATTR(termios_exp, termios, c_oflag);
+	if (!flag)
+		tst_res(TPASS, "TCGETS/TCSETS tests");
+}
+
 static void do_child(void)
 {
 	int cfd = SAFE_OPEN(childtty, O_RDWR, 0777);
@@ -187,14 +231,16 @@ static void setup(void)
 
 	int fd = SAFE_OPEN(device, O_RDWR, 0777);
 
-	SAFE_IOCTL(fd, TCGETA, &save_io);
+	SAFE_IOCTL(fd, TCGETS, &termios_bak);
 	SAFE_CLOSE(fd);
+
+	prepare_termio();
 }
 
 static void cleanup(void)
 {
 	if (parentfd >= 0) {
-		SAFE_IOCTL(parentfd, TCSETA, &save_io);
+		SAFE_IOCTL(parentfd, TCSETS, &termios_bak);
 		SAFE_CLOSE(parentfd);
 	}
 }
@@ -206,6 +252,7 @@ static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_all = verify_ioctl,
+	.test_variants = 2,
 	.options = (struct tst_option[]) {
 		{"D:", &device, "Tty device. For example, /dev/tty[0-9]"},
 		{}
-- 
2.42.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v9 4/4] Extend ioctl02 to test termio and termios

[Marius Kittler]

Am Freitag, 27. Oktober 2023, 16:58:58 CET schrieb Cyril Hrubis:
> Hi!
> 
> > +static void chk_tty_parms_termio(void)
> > +{
> > +	int i, flag = 0;
> > 
> > +	CMP_ATTR(termio_exp, termio, c_line);
> > +	CHECK_CONTROL_CHARS(termio);
> > +	CMP_ATTR(termio_exp, termio, c_lflag);
> > +	CMP_ATTR(termio_exp, termio, c_iflag);
> > +	CMP_ATTR(termio_exp, termio, c_oflag);
> 
> I do not see a reason why this cannot be a function, the only difference
> would be that we would have to do cmp_attr(termio_exp, termio->c_line)
> instead.

I'm not sure whether I understand this suggestion. If the first parameter is 
still just termio_exp, how would that function know what field in termio_exp to 
compare with? I guess you had an invocation like this in mind:

cmp_attr(termio_exp->c_line, termio->c_line)

That would be possible. The repetition of the attribute name is not nice. Of 
course we'd also lose the attribute name in the error message. This could be 
solved by passing it as string and doing a dynamic string concatenation in the 
function, so the invocation would look like this:

cmp_attr(termio_exp->c_line, termio->c_line, "c_line")

This would also mean two additional implicit type conversions per call of this 
function within chk_tty_parms_termio() because termio uses smaller sized types 
than termios and the cmp_attr() function would need to decide on one type (and 
we'd have to pick the larger size).

So I'm honestly not sure whether any of this is better than just making it a 
macro or maybe I don't understand your suggestion.



-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v9 4/4] Extend ioctl02 to test termio and termios

[Cyril Hrubis]

Hi!
> So I'm honestly not sure whether any of this is better than just making it a 
> macro or maybe I don't understand your suggestion.

And that what macros are for, avoiding repetitive parameters, my
soulution would be to add a macro that just prepares the parameters
for the function and then keep all the code in a function, i.e.

#define CMP_ATTR(term_exp, term, attr) \
        cmp_attr((term_exp)->attr, (term)->attr, #attr)

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v9 4/4] Extend ioctl02 to test termio and termios

[Marius Kittler]

Am Donnerstag, 2. November 2023, 14:26:17 CET schrieb Cyril Hrubis:
> And that what macros are for, avoiding repetitive parameters, my
> soulution would be to add a macro that just prepares the parameters
> for the function and then keep all the code in a function, i.e.
> 
> #define CMP_ATTR(term_exp, term, attr) \
>         cmp_attr((term_exp)->attr, (term)->attr, #attr)

That makes sense and turned out to be a very good suggestion, indeed. I 
implemented this change and could even apply it to the other macro (after a 
bit of simplification). I pushed this change as a new patch set because the 
preceding patches of this one have already been merged.



-- 
Mailing list info: https://lists.linux.it/listinfo/ltp