[LTP] [PATCH v1] Extend ioctl02 to test termio and termios

Martin Doucha mdoucha@suse.cz
Thu Nov 2 17:35:24 CET 2023


Hi,
the changes look good, just 3 small suggestions below that can be added 
during merge. Otherwise:

Reviewed-by: Martin Doucha <mdoucha@suse.cz>

On 02. 11. 23 16:24, Marius Kittler wrote:
> Testing the behavior of both sets of ioctl commands at the same time is
> challenging because they use different structures. This change tries to
> minimize the amount of code duplication (it is not fully possible) and to
> minimize the amount of macro code (it is not fully avoidable).
> 
> To ease this, this change simplifies the checks:
> * Create a separate function and macro for checking attributes
> * Remove useless nested condition in loop for checking the control
>    characters
>      * The loop condition is `i < NCC` and the nested condition was
>        `i == VEOL2`. The nested condition is completely useless because it
>        is never reached because `VEOL2 > NCC`.
>      * The same applies to the loop for initializing the structs.
>      * This change introduces use of termios (where `VEOL2 < NCCS`) but
>        extra handling for `VEOL2` can still be avoided.
> * Implement the check for control characters in terms of the normal
>    attribute check
> 
> Signed-off-by: Marius Kittler <mkittler@suse.de>
> ---
>   testcases/kernel/syscalls/ioctl/ioctl02.c | 190 +++++++++++++---------
>   1 file changed, 117 insertions(+), 73 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl02.c b/testcases/kernel/syscalls/ioctl/ioctl02.c
> index 8c6924386..333cad375 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,109 @@ static void verify_ioctl(void)
>   	run_ptest();
>   
>   	TST_CHECKPOINT_WAKE(0);
> +
> +	if (tst_variant < sizeof(variants) - 1)
> +		SAFE_CLOSE(parentfd);

The condition above is not needed. SAFE_CLOSE() will automatically set 
parentfd = -1; so cleanup() will not try to close it again.

>   }
>   
> -/*
> - * 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;
> -	}
> +	for (int i = 0; i < NCC; i++)
> +		termio_exp.c_cc[i] = CSTART;
> +	for (int i = 0; i < VEOL2; i++)
> +		termios_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;
> +	termios_exp.c_oflag = termio_exp.c_oflag = OPOST | OLCUC | ONLCR | ONOCR;
> +
> +	/* Init termio/termios structures used to check if all params got set */
> +	memset(&termio, 0, sizeof(termio));
> +	memset(&termios, 0, sizeof(termios));

These memset()s should be done in run_ptest(). If the test is run with 
multiple iterations using the -i or -I parameters, setup() is called 
only once and the variables would keep stale data after the first iteration.

> +}
> +
> +/*
> + * 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];
>   
> -	SAFE_IOCTL(parentfd, TCSETA, &termio);
> +	SAFE_IOCTL(parentfd, v->tcset, v->termio_exp);
>   
>   	/* Get termio and see if all parameters actually got set */
> -	SAFE_IOCTL(parentfd, TCGETA, &termio);
> -	chk_tty_parms();
> +	SAFE_IOCTL(parentfd, v->tcget, v->termio);
> +	v->check();
>   }
>   
> -static void chk_tty_parms(void)
> +static int cmp_attr(unsigned long long exp, unsigned long long act, const char *attr)
>   {
> -	int i, flag = 0;
> +	if (act == exp)
> +		return 0;
> +	tst_res(TFAIL, "%s has incorrect value %llu", attr, act);

Printing the expected value would be useful for debugging failures.

> +	return 1;
> +}
>   
> -	if (termio.c_line != 0) {
> -		tst_res(TFAIL, "line discipline has incorrect value %o",
> -			 termio.c_line);
> -		flag++;
> -	}
> +static int cmp_c_cc(unsigned char *exp_c_cc, unsigned char *act_c_cc, int ncc)
> +{
> +	int i, fails = 0;
> +	char what[32];
>   
> -	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++;
> -		}
> +	for (i = 0; i < ncc; ++i) {
> +		sprintf(what, "control char %d", i);
> +		fails += cmp_attr(exp_c_cc[i], act_c_cc[i], what);
>   	}
> +	return fails;
> +}
>   
> -	if (termio.c_lflag != (ISIG | ICANON | XCASE | ECHO | ECHOE
> -		 | NOFLSH)) {
> -		tst_res(TFAIL, "lflag has incorrect value. %o",
> -			 termio.c_lflag);
> -		flag++;
> -	}
> +#define CMP_ATTR(term_exp, term, attr) \
> +	(flag += cmp_attr((term_exp).attr, (term).attr, #attr))
>   
> -	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_C_CC(term_exp, term) \
> +	(flag += cmp_c_cc(term_exp.c_cc, term.c_cc, sizeof(term.c_cc)))
>   
> -	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 flag = 0;
>   
> +	CMP_ATTR(termio_exp, termio, c_line);
> +	CMP_C_CC(termio_exp, 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 flag = 0;
> +
> +	CMP_ATTR(termios_exp, termios, c_line);
> +	CMP_C_CC(termios_exp, 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 +228,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 +249,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