[LTP] [PATCH v7 5/5] Extend ioctl02 to test termio and termios

Martin Doucha mdoucha@suse.cz
Tue Oct 24 17:54:27 CEST 2023


Hi,
I'd strongly recommend merging this patch with the previous one. There's 
no point switching everything to termios only to add termio back in the 
next commit.

On 23. 10. 23 18:02, Marius Kittler wrote:
> Signed-off-by: Marius Kittler <mkittler@suse.de>
> ---
>   testcases/kernel/syscalls/ioctl/ioctl02.c | 170 +++++++++++++++++++---
>   1 file changed, 147 insertions(+), 23 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl02.c b/testcases/kernel/syscalls/ioctl/ioctl02.c
> index 5111d7178..00d51b367 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl02.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl02.c
> @@ -8,22 +8,24 @@
>   /*\
>    * [Description]
>    *
> - * Testcase to test the TCGETS, and TCSETS 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 TCGETS ioctl to get all
> - * the tty parameters. It then changes them to known values by issuing a
> - * TCSETS ioctl. Then the parent issues a TCGETS ioctl again and compares
> - * the received values with what it had set earlier. The test fails if
> - * TCGETS or 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.
> + * 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 <asm-generic/ioctls.h>

#including <asm/*> or <asm-generic/*> directly is generally a bad 
practice. You should #include <sys/ioctl.h> instead.

>   #include <stdio.h>
>   #include <fcntl.h>
>   #include <errno.h>
> @@ -38,23 +40,54 @@
>   #include "tst_test.h"
>   #include "tst_safe_macros.h"
>   
> -static struct termios termio, save_io;
> +static struct termio termio;
> +static struct termios termios, termios_bak;
>   
>   static char *parenttty, *childtty;
>   static int parentfd = -1, childfd = -1;
>   static int parentpid, childpid;
>   
>   static int do_child_setup(void);
> +static void prepare_termio(void);
> +static void preapre_termios(void);
>   static void run_ptest(void);
>   static void run_ctest(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_bak;
> +	void (*prepare)(void);
> +	void (*check)(void);
> +	int tcget, tcset;
> +} variants[] = {
> +	{
> +		.name = "termio",
> +		.termio = &termio,
> +		.prepare = &prepare_termio,
> +		.check = &chk_tty_parms_termio,
> +		.tcget = TCGETA,
> +		.tcset = TCSETA,
> +	},
> +	{
> +		.name = "termios",
> +		.termio = &termios,
> +		.prepare = preapre_termios,
> +		.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;
>   
> @@ -80,11 +113,7 @@ static void verify_ioctl(void)
>   	TST_CHECKPOINT_WAKE(0);
>   }
>   
> -/*
> - * run_ptest() - setup the various termio structure values and issue
> - *		 the TCSETS ioctl call with the TEST macro.
> - */
> -static void run_ptest(void)
> +static void prepare_termio(void)
>   {
>   	/* Use "old" line discipline */
>   	termio.c_line = 0;
> @@ -110,12 +139,50 @@ static void run_ptest(void)
>   
>   	/* Set output modes. */
>   	termio.c_oflag = OPOST | OLCUC | ONLCR | ONOCR;
> +}
>   
> -	SAFE_IOCTL(parentfd, TCSETS, &termio);
> +static void preapre_termios(void)
> +{
> +	/* Use "old" line discipline */
> +	termios.c_line = 0;

You don't need to duplicate the whole prepare_termio() function. You can 
either initialize both structures at once:
termio.c_line = termios.c_line = 0;
termio.c_cflag = termios.c_cflag = ...;

of #define an init macro:
#define TERM_INIT_VALUES { .c_line = 0, .c_cflag = ... }
termio = (struct termio)TERM_INIT_VALUES;
termios = (struct termios)TERM_INIT_VALUES;

> +
> +	/* Set control modes */
> +	termios.c_cflag = B50 | CS7 | CREAD | PARENB | PARODD | CLOCAL;
> +
> +	/* Set control chars. */
> +	for (int i = 0; i < NCC; i++) {
> +		if (i == VEOL2)
> +			continue;
> +		termios.c_cc[i] = CSTART;
> +	}
> +
> +	/* Set local modes. */
> +	termios.c_lflag =
> +	    ((unsigned short)(ISIG | ICANON | XCASE | ECHO | ECHOE | NOFLSH));
> +
> +	/* Set input modes. */
> +	termios.c_iflag =
> +	    BRKINT | IGNPAR | INPCK | ISTRIP | ICRNL | IUCLC | IXON | IXANY |
> +	    IXOFF;
> +
> +	/* Set output modes. */
> +	termios.c_oflag = OPOST | OLCUC | ONLCR | ONOCR;
> +}
> +
> +/*
> + * 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)
> +{
> +	struct variant *v = &variants[tst_variant];
> +
> +	v->prepare();
> +	SAFE_IOCTL(parentfd, v->tcset, v->termio);
>   
>   	/* Get termio and see if all parameters actually got set */
> -	SAFE_IOCTL(parentfd, TCGETS, &termio);
> -	chk_tty_parms();
> +	SAFE_IOCTL(parentfd, v->tcget, v->termio);
> +	v->check();
>   }
>   
>   static void run_ctest(void)
> @@ -125,7 +192,7 @@ static void run_ctest(void)
>   	SAFE_CLOSE(childfd);
>   }
>   
> -static void chk_tty_parms(void)
> +static void chk_tty_parms_termio(void)
>   {
>   	int i, flag = 0;
>   
> @@ -174,6 +241,62 @@ static void chk_tty_parms(void)
>   		flag++;
>   	}
>   
> +	if (flag != 0)
> +		tst_res(TFAIL, "TCGETA/TCSETA tests FAILED with "
> +				"%d %s", flag, flag > 1 ? "errors" : "error");
> +	else
> +		tst_res(TPASS, "TCGETA/TCSETA tests SUCCEEDED");
> +}
> +
> +static void chk_tty_parms_termios(void)
> +{
> +	int i, flag = 0;
> +
> +	if (termios.c_line != 0) {
> +		tst_res(TINFO, "line discipline has incorrect value %o",
> +			 termios.c_line);
> +		flag++;
> +	}

Some code duplication is unavoidable here but the value validation can 
be simplified:

1) Add another pair of termio+termios variables which will be 
initialized in setup() and passed only to SAFE_IOCTL(TCSETA/TCSETS).

2) memset() the existing termio+termios variables before passing them to 
SAFE_IOCTL(TCGETA/TCGETS). As is, the test can pass even if 
TCGETA/TCGETS does nothing because they store data into a structure that 
already contains the exact same values.

3) Define a macro to compare the "set" and "get" variables:
#define CMP_ATTR(tcexp, tcval, attr) \
	if ((tcval).attr != (tcexp).attr) { \
		tst_res(TINFO, #attr " has incorrect value %o", \
			 (tcval).attr); \
		flag++; \
	}

Then all attribtute checks except the loop can be reduced to one line.

> +
> +	for (i = 0; i < NCC; i++) {
> +		if (i == VEOL2) {
> +			if (!termios.c_cc[i]) {
> +				continue;
> +			} else {
> +				tst_res(TINFO, "control char %d has "
> +					 "incorrect value %d", i, termios.c_cc[i]);
> +				flag++;
> +				continue;
> +			}
> +		}
> +
> +		if (termios.c_cc[i] != CSTART) {
> +			tst_res(TINFO, "control char %d has incorrect "
> +				 "value %d.", i, termios.c_cc[i]);
> +			flag++;
> +		}
> +	}
> +
> +	if (termios.c_lflag != (ISIG | ICANON | XCASE | ECHO | ECHOE
> +		 | NOFLSH)) {
> +		tst_res(TINFO, "lflag has incorrect value. %o",
> +			 termios.c_lflag);
> +		flag++;
> +	}
> +
> +	if (termios.c_iflag != (BRKINT | IGNPAR | INPCK | ISTRIP
> +		 | ICRNL | IUCLC | IXON | IXANY | IXOFF)) {
> +		tst_res(TINFO, "iflag has incorrect value. %o",
> +			 termios.c_iflag);
> +		flag++;
> +	}
> +
> +	if (termios.c_oflag != (OPOST | OLCUC | ONLCR | ONOCR)) {
> +		tst_res(TINFO, "oflag has incorrect value. %o",
> +			 termios.c_oflag);
> +		flag++;
> +	}
> +
>   	if (flag != 0)
>   		tst_res(TFAIL, "TCGETS/TCSETS tests FAILED with "
>   				"%d %s", flag, flag > 1 ? "errors" : "error");
> @@ -202,7 +325,7 @@ static void setup(void)
>   	int fd = SAFE_OPEN(device, O_RDWR, 0777);
>   
>   	/* Save the current device information - to be restored in cleanup() */
> -	SAFE_IOCTL(fd, TCGETS, &save_io);
> +	SAFE_IOCTL(fd, TCGETS, &termios_bak);
>   
>   	/* Close the device */
>   	SAFE_CLOSE(fd);
> @@ -211,7 +334,7 @@ static void setup(void)
>   static void cleanup(void)
>   {
>   	if (parentfd >= 0) {
> -		SAFE_IOCTL(parentfd, TCSETS, &save_io);
> +		SAFE_IOCTL(parentfd, TCSETS, &termios_bak);
>   		SAFE_CLOSE(parentfd);
>   	}
>   }
> @@ -223,6 +346,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]"},
>   		{}

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic



More information about the ltp mailing list