[LTP] [PATCH] Use correct ioctl request for termios

[LTP] [PATCH] Use correct ioctl request for termios

[Marius Kittler]

With the termios struct one is supposed to use TCGETS instead of TCGETA,
see `man ioctl_tty`.

Related issue: https://github.com/linux-test-project/ltp/issues/637#issuecomment-1705334874
---
 testcases/kernel/syscalls/ioctl/ioctl01.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/testcases/kernel/syscalls/ioctl/ioctl01.c b/testcases/kernel/syscalls/ioctl/ioctl01.c
index 1be38e79d..983208925 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl01.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl01.c
@@ -36,34 +36,35 @@ static struct termios termios;
 
 static struct tcase {
 	int *fd;
-	int request;
+	int request_tio;
+	int request_tios;
 	struct termio *s_tio;
 	struct termios *s_tios;
 	int error;
 } tcases[] = {
 	/* file descriptor is invalid */
-	{&bfd, TCGETA, &termio, &termios, EBADF},
+	{&bfd, TCGETA, TCGETS, &termio, &termios, EBADF},
 	/* termio address is invalid */
-	{&fd, TCGETA, (struct termio *)-1, (struct termios *)-1, EFAULT},
-	/* command is invalid */
+	{&fd, TCGETA, TCGETS, (struct termio *)-1, (struct termios *)-1, EFAULT},
+	// /* command is invalid */
 	/* This errno value was changed from EINVAL to ENOTTY
 	 * by kernel commit 07d106d0 and bbb63c51
 	 */
-	{&fd, INVAL_IOCTL, &termio, &termios, ENOTTY},
+	{&fd, INVAL_IOCTL, INVAL_IOCTL, &termio, &termios, ENOTTY},
 	/* file descriptor is for a regular file */
-	{&fd_file, TCGETA, &termio, &termios, ENOTTY},
+	{&fd_file, TCGETA, TCGETS, &termio, &termios, ENOTTY},
 	/* termio is NULL */
-	{&fd, TCGETA, NULL, NULL, EFAULT}
+	{&fd, TCGETA, TCGETS, NULL, 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),
+	TST_EXP_FAIL(ioctl(*(tcases[i].fd), tcases[i].request_tio, tcases[i].s_tio),
 		     tcases[i].error);
 
-	TST_EXP_FAIL(ioctl(*(tcases[i].fd), tcases[i].request, tcases[i].s_tios),
+	TST_EXP_FAIL(ioctl(*(tcases[i].fd), tcases[i].request_tios, tcases[i].s_tios),
 		     tcases[i].error);
 }
 
-- 
2.41.0


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

Re: [LTP] [PATCH] Use correct ioctl request for termios

[Cyril Hrubis]

Hi!
>  testcases/kernel/syscalls/ioctl/ioctl01.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl01.c b/testcases/kernel/syscalls/ioctl/ioctl01.c
> index 1be38e79d..983208925 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl01.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl01.c
> @@ -36,34 +36,35 @@ static struct termios termios;
>  
>  static struct tcase {
>  	int *fd;
> -	int request;
> +	int request_tio;
> +	int request_tios;
>  	struct termio *s_tio;
>  	struct termios *s_tios;
>  	int error;

I guess that it would be cleaner to keep the single request, but define
the s_tio as a void * here.

That way we can add test on a separate lines:

	{&bfd, TCGETA, &termio, EBADF},
	{&bfd, TCGETS, &termios, EBADF},
	...


And have single TST_EXP_FAIL() in the verify function.

>  } tcases[] = {
>  	/* file descriptor is invalid */
> -	{&bfd, TCGETA, &termio, &termios, EBADF},
> +	{&bfd, TCGETA, TCGETS, &termio, &termios, EBADF},
>  	/* termio address is invalid */
> -	{&fd, TCGETA, (struct termio *)-1, (struct termios *)-1, EFAULT},
> -	/* command is invalid */
> +	{&fd, TCGETA, TCGETS, (struct termio *)-1, (struct termios *)-1, EFAULT},
> +	// /* command is invalid */
>  	/* This errno value was changed from EINVAL to ENOTTY
>  	 * by kernel commit 07d106d0 and bbb63c51
>  	 */
> -	{&fd, INVAL_IOCTL, &termio, &termios, ENOTTY},
> +	{&fd, INVAL_IOCTL, INVAL_IOCTL, &termio, &termios, ENOTTY},
>  	/* file descriptor is for a regular file */
> -	{&fd_file, TCGETA, &termio, &termios, ENOTTY},
> +	{&fd_file, TCGETA, TCGETS, &termio, &termios, ENOTTY},
>  	/* termio is NULL */
> -	{&fd, TCGETA, NULL, NULL, EFAULT}
> +	{&fd, TCGETA, TCGETS, NULL, 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),
> +	TST_EXP_FAIL(ioctl(*(tcases[i].fd), tcases[i].request_tio, tcases[i].s_tio),
>  		     tcases[i].error);
>  
> -	TST_EXP_FAIL(ioctl(*(tcases[i].fd), tcases[i].request, tcases[i].s_tios),
> +	TST_EXP_FAIL(ioctl(*(tcases[i].fd), tcases[i].request_tios, tcases[i].s_tios),
>  		     tcases[i].error);
>  }
>  
> -- 
> 2.41.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH] Use correct ioctl request for termios

[Marius Kittler]

Am Montag, 4. September 2023, 17:04:51 CEST schrieb Cyril Hrubis:
> 
> I guess that it would be cleaner to keep the single request, but define
> the s_tio as a void * here.
> 
> That way we can add test on a separate lines:
> 
> 	{&bfd, TCGETA, &termio, EBADF},
> 	{&bfd, TCGETS, &termios, EBADF},
> 	...
> 

But we have currently 4 tests that are all executed via both termio and 
termios. To keep this kind of coverage we would need to duplicate all of those 
lines. That would be 4 new test lines in total. Is that what you're 
suggesting? I'm just asking twice because I'm not sure whether it is better 
(but don't have a strong opinion).

By the way, how would I submit an amendment here? Just submit the amended 
commit again in the same way (I used `git send-email --
to=ltp@lists.linux.it`)?

And how is this merged? I saw that in another case patchwork was used? How 
does that come into play?



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

Re: [LTP] [PATCH] Use correct ioctl request for termios

[Cyril Hrubis]

Hi!
> But we have currently 4 tests that are all executed via both termio and 
> termios. To keep this kind of coverage we would need to duplicate all of those 
> lines. That would be 4 new test lines in total. Is that what you're 
> suggesting? I'm just asking twice because I'm not sure whether it is better 
> (but don't have a strong opinion).

Yes, as far as I can tell one test per entry is much cleaner and easier
to read.

> By the way, how would I submit an amendment here? Just submit the amended 
> commit again in the same way (I used `git send-email --
> to=ltp@lists.linux.it`)?

Yes, and you are supposed to version the commits, so add to format-patch
-v2 for second version, etc.

> And how is this merged? I saw that in another case patchwork was used? How 
> does that come into play?

Things get merged by maintainers who apply the patch to their local git
tree and then push to github. Patchwork is just a tool that monitors the
mailing list and saves a list of patches that needs review, which makes
things easier for developers as it's easy for patches to get burried in
their mailboxes.

-- 
Cyril Hrubis
chrubis@suse.cz

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