| Message ID | 20230129183930.2045-1-wegao@suse.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v1] ioctl01.c:Test also struct termios | expand |
On Mon, Jan 30, 2023 at 12:03 PM Wei Gao via ltp <ltp@lists.linux.it> wrote: > ATM we're testing just legacy struct termio in ioctl01.c, > we also need test struct termios. > > Signed-off-by: Wei Gao <wegao@suse.com> > Reviewed-by: Li Wang <liwang@redhat.com> > --- > testcases/kernel/syscalls/ioctl/ioctl01.c | 28 ++++++++++++++++++----- > 1 file changed, 22 insertions(+), 6 deletions(-) > > diff --git a/testcases/kernel/syscalls/ioctl/ioctl01.c > b/testcases/kernel/syscalls/ioctl/ioctl01.c > index 2989c0e9b..cc8d1d731 100644 > --- a/testcases/kernel/syscalls/ioctl/ioctl01.c > +++ b/testcases/kernel/syscalls/ioctl/ioctl01.c > @@ -28,26 +28,28 @@ static int fd, fd_file; > static int bfd = -1; > > static struct termio termio; > +static struct termios termios; > > static struct tcase { > int *fd; > int request; > struct termio *s_tio; > + struct termios *s_tios; > int error; > } tcases[] = { > /* file descriptor is invalid */ > - {&bfd, TCGETA, &termio, EBADF}, > + {&bfd, TCGETA, &termio, &termios, EBADF}, > /* termio address is invalid */ > - {&fd, TCGETA, (struct termio *)-1, EFAULT}, > + {&fd, TCGETA, (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, ENOTTY}, > + {&fd, INVAL_IOCTL, &termio, &termios, ENOTTY}, > /* file descriptor is for a regular file */ > - {&fd_file, TCGETA, &termio, ENOTTY}, > + {&fd_file, TCGETA, &termio, &termios, ENOTTY}, > /* termio is NULL */ > - {&fd, TCGETA, NULL, EFAULT} > + {&fd, TCGETA, NULL, NULL, EFAULT} > }; > > static char *device; > @@ -64,7 +66,21 @@ static void verify_ioctl(unsigned int i) > if (TST_ERR != tcases[i].error) { > tst_res(TFAIL | TTERRNO, > "failed unexpectedly; expected %s", > - tst_strerrno(tcases[i].error)); > + tst_strerrno(tcases[i].error)); > + return; > + } > + > + TEST(ioctl(*(tcases[i].fd), tcases[i].request, tcases[i].s_tios)); > + > + if (TST_RET != -1) { > + tst_res(TFAIL, "call succeeded unexpectedly"); > + return; > + } > + > + if (TST_ERR != tcases[i].error) { > + tst_res(TFAIL | TTERRNO, > + "failed unexpectedly; expected %s", > + tst_strerrno(tcases[i].error)); > return; > } > > -- > 2.35.3 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp > >
Hi Wei, > ATM we're testing just legacy struct termio in ioctl01.c, > we also need test struct termios. Good idea. > @@ -64,7 +66,21 @@ static void verify_ioctl(unsigned int i) > if (TST_ERR != tcases[i].error) { > tst_res(TFAIL | TTERRNO, > "failed unexpectedly; expected %s", > - tst_strerrno(tcases[i].error)); > + tst_strerrno(tcases[i].error)); > + return; > + } Could you please replace TEST(ioctl(...) with TST_EXP_FAIL(ioctl(...))? That would save all error handling. We try to use these helping macros in include/tst_test_macros.h. Although they would deserve some docs as macros are a bit harder to read. > + > + TEST(ioctl(*(tcases[i].fd), tcases[i].request, tcases[i].s_tios)); > + And obviously here should go TST_EXP_FAIL(ioctl(...)) as well. Kind regards, Petr > + if (TST_RET != -1) { > + tst_res(TFAIL, "call succeeded unexpectedly"); > + return; > + } > + > + if (TST_ERR != tcases[i].error) { > + tst_res(TFAIL | TTERRNO, > + "failed unexpectedly; expected %s", > + tst_strerrno(tcases[i].error)); > return; > }
Hi Wei, > Hi Wei, ... > > @@ -64,7 +66,21 @@ static void verify_ioctl(unsigned int i) > > if (TST_ERR != tcases[i].error) { > > tst_res(TFAIL | TTERRNO, > > "failed unexpectedly; expected %s", > > - tst_strerrno(tcases[i].error)); > > + tst_strerrno(tcases[i].error)); > > + return; > > + } > Could you please replace TEST(ioctl(...) with TST_EXP_FAIL(ioctl(...))? > That would save all error handling. > We try to use these helping macros in include/tst_test_macros.h. > Although they would deserve some docs as macros are a bit harder to read. I implemented this in 4c86809f77 ("ioctl01: cleanup"). Could you please rebase your commit and use TST_EXP_FAIL() in it? Thank you! Kind regards, Petr
On Mon, Feb 06, 2023 at 07:12:11AM +0100, Petr Vorel wrote: > Hi Wei, > > > Hi Wei, > > ... > > > @@ -64,7 +66,21 @@ static void verify_ioctl(unsigned int i) > > > if (TST_ERR != tcases[i].error) { > > > tst_res(TFAIL | TTERRNO, > > > "failed unexpectedly; expected %s", > > > - tst_strerrno(tcases[i].error)); > > > + tst_strerrno(tcases[i].error)); > > > + return; > > > + } > > Could you please replace TEST(ioctl(...) with TST_EXP_FAIL(ioctl(...))? > > That would save all error handling. > > > We try to use these helping macros in include/tst_test_macros.h. > > Although they would deserve some docs as macros are a bit harder to read. > > I implemented this in 4c86809f77 ("ioctl01: cleanup"). Could you please rebase > your commit and use TST_EXP_FAIL() in it? Done, could you help check it, thanks! > > Thank you! > > Kind regards, > Petr
diff --git a/testcases/kernel/syscalls/ioctl/ioctl01.c b/testcases/kernel/syscalls/ioctl/ioctl01.c index 2989c0e9b..cc8d1d731 100644 --- a/testcases/kernel/syscalls/ioctl/ioctl01.c +++ b/testcases/kernel/syscalls/ioctl/ioctl01.c @@ -28,26 +28,28 @@ static int fd, fd_file; static int bfd = -1; static struct termio termio; +static struct termios termios; static struct tcase { int *fd; int request; struct termio *s_tio; + struct termios *s_tios; int error; } tcases[] = { /* file descriptor is invalid */ - {&bfd, TCGETA, &termio, EBADF}, + {&bfd, TCGETA, &termio, &termios, EBADF}, /* termio address is invalid */ - {&fd, TCGETA, (struct termio *)-1, EFAULT}, + {&fd, TCGETA, (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, ENOTTY}, + {&fd, INVAL_IOCTL, &termio, &termios, ENOTTY}, /* file descriptor is for a regular file */ - {&fd_file, TCGETA, &termio, ENOTTY}, + {&fd_file, TCGETA, &termio, &termios, ENOTTY}, /* termio is NULL */ - {&fd, TCGETA, NULL, EFAULT} + {&fd, TCGETA, NULL, NULL, EFAULT} }; static char *device; @@ -64,7 +66,21 @@ static void verify_ioctl(unsigned int i) if (TST_ERR != tcases[i].error) { tst_res(TFAIL | TTERRNO, "failed unexpectedly; expected %s", - tst_strerrno(tcases[i].error)); + tst_strerrno(tcases[i].error)); + return; + } + + TEST(ioctl(*(tcases[i].fd), tcases[i].request, tcases[i].s_tios)); + + if (TST_RET != -1) { + tst_res(TFAIL, "call succeeded unexpectedly"); + return; + } + + if (TST_ERR != tcases[i].error) { + tst_res(TFAIL | TTERRNO, + "failed unexpectedly; expected %s", + tst_strerrno(tcases[i].error)); return; }
ATM we're testing just legacy struct termio in ioctl01.c, we also need test struct termios. Signed-off-by: Wei Gao <wegao@suse.com> --- testcases/kernel/syscalls/ioctl/ioctl01.c | 28 ++++++++++++++++++----- 1 file changed, 22 insertions(+), 6 deletions(-)