Re: [LTP] [PATCH v1] Avoid messing with system tty in ioctl01.c

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> * Use `openpty()` to create a new tty
> * Remove `-D` option
> * Remove requirement to run as root
> 
> Signed-off-by: Marius Kittler <mkittler@suse.de>
> ---
>  runtest/syscalls                           |  2 +-
>  testcases/kernel/syscalls/ioctl/ioctl01.c  | 25 ++++++++++------------
>  testcases/kernel/syscalls/ioctl/test_ioctl | 23 --------------------
>  3 files changed, 12 insertions(+), 38 deletions(-)
> 
> diff --git a/runtest/syscalls b/runtest/syscalls
> index b1125dd75..f999bd74f 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -557,10 +557,10 @@ init_module01 init_module01
>  init_module02 init_module02
>  
>  #Needs tty device.
> -#ioctl01 ioctl01 -D /dev/tty0
>  #ioctl02 ioctl02 -D /dev/tty0
>  
>  # Introducing ioctl tests for all /dev/tty* devices
> +ioctl01      ioctl01
>  ioctl01_02   test_ioctl

This should be renamed to ioctl02 now I suppose.

>  ioctl03      ioctl03
>  ioctl04      ioctl04
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl01.c b/testcases/kernel/syscalls/ioctl/ioctl01.c
> index 3ca8a9a3b..fe6a5591d 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl01.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl01.c
> @@ -23,11 +23,13 @@
>  #include <fcntl.h>
>  #include <stdio.h>
>  #include <termios.h>
> +#include <pty.h>
>  #include "tst_test.h"
>  #include "lapi/ioctl.h"
>  
>  #define	INVAL_IOCTL	9999999
>  
> +static int amaster, aslave;
>  static int fd, fd_file;
>  static int bfd = -1;
>  
> @@ -59,8 +61,6 @@ static struct tcase {
>  	{&fd, TCGETS, NULL, EFAULT}
>  };
>  
> -static char *device;
> -
>  static void verify_ioctl(unsigned int i)
>  {
>  	TST_EXP_FAIL(ioctl(*(tcases[i].fd), tcases[i].request, tcases[i].s_tio),
> @@ -69,31 +69,28 @@ static void verify_ioctl(unsigned int i)
>  
>  static void setup(void)
>  {
> -	if (!device)
> -		tst_brk(TBROK, "You must specify a tty device with -D option");
> +	if (openpty(&amaster, &aslave, NULL, &termios, NULL) < 0) {
> +		tst_brk(TBROK | TERRNO, "unable to open pty");
> +	}

It's better just to pass NULL instead of the zero filled &termios here.

The rest looks good to me, if you agree I can merge the patch with the
changes I've proposed.

-- 
Cyril Hrubis
chrubis@suse.cz

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