[PATCH 6/6] tests: add a test case for the abitrator of ioctl command overwrapping

[Dmitry V. Levin]

On Thu, Mar 10, 2022 at 09:01:06PM +0900, Masatake YAMATO wrote:
[...]
> diff --git a/tests/ioctl_overwrap.c b/tests/ioctl_overwrap.c
> new file mode 100644
> index 000000000..aa66aa715
> --- /dev/null
> +++ b/tests/ioctl_overwrap.c
> @@ -0,0 +1,63 @@
> +/*
> + * Test the code arbitrating the overlapped IOCTL commands

Sorry, I'm a bit confused: the test is called ioctl_overwrap,
and the description talks about overlapped IOCTL commands.

> +#define _XOPEN_SOURCE 600

You don't need this due to use of AC_USE_SYSTEM_EXTENSIONS in
configure.ac.

> +int
> +main(int argc, char **argv)
> +{

If the test needs a mounted /proc filesystem, please add
	skip_if_unavailable("/proc/self/fd/");
in the beginning of the test.

> +	struct termios tos;
> +	memset(&tos, 0, sizeof(tos));
> +
> +	int ptm = posix_openpt(O_RDWR);
> +	if (ptm < 0)
> +		perror_msg_and_fail("posix_openpt(_RDWR)");

This shouldn't be fatal, please use perror_msg_and_skip instead.
By the way, what is "_RDWR"?

> +
> +	if (unlockpt(ptm) < 0)
> +		perror_msg_and_fail("unlockpt(%d)", ptm);

Likewise, please don't treat this error as fatal.

> +	printf("ioctl(%d, TIOCSPTLCK, [0]) = 0\n", ptm);

If you expect unlockpt to call TIOCSPTLCK, wouldn't it be better to call
TIOCSPTLCK directly?

> +
> +	char *slvname = ptsname(ptm);
> +	if (!slvname)
> +		error_msg_and_fail("ptsname(%d) returns NULL", ptm);
> +	char *n = strrchr(slvname, '/');
> +	if (!n)
> +		error_msg_and_fail("ptsname returns unexpected string: %s\n",
> +				   slvname);
> +	/* slvname may be /dev/pts/[0-9]+. */
> +	printf("ioctl(%d, TIOCGPTN, [%s]) = 0\n", ptm, n + 1);
> +
> +	int pts = open(slvname, O_RDWR);
> +	if (pts < 0)
> +		perror_msg_and_fail("open(%s)", slvname);
> +
> +	tcsetattr(pts, TCSANOW, &tos);
> +	printf("ioctl(%d, TCSETS, {B0 -opost -isig -icanon -echo ...}) = 0\n", pts);
> +	printf("ioctl(%d, TCGETS, {B0 -opost -isig -icanon -echo ...}) = 0\n", pts);

Likewise, if you expect tcsetattr to call TCSETS and TCGETS,
wouldn't it be better to call them directly?
By the way, this is architecture-specific, on some architectures
TCSETS2/TCGETS2 are used instead.

> +	int devnull = open("/dev/null", O_RDWR);
> +	if (pts < 0)
> +		perror_msg_and_fail("open(/dev/null)");
> +
> +	tcsetattr(devnull, TCSANOW, &tos);
> +	printf("ioctl(%d, SNDCTL_TMR_START or TCSETS, {B0 -opost -isig -icanon -echo ...}) = -1 ENOTTY (Inappropriate ioctl for device)\n",
> +	       devnull);

Consider using sprintrc(-1) instead of open-coding the error code and
text.


-- 
ldv