Message ID | 20220715061548.11574-1-akumar@suse.de |
---|---|
State | Superseded |
Headers | show |
Series | unlink07: use TST_EXP_FAIL() and turn comment into docparse, reword | expand |
Hi Avinesh, > +++ b/testcases/kernel/syscalls/unlink/unlink07.c > @@ -3,15 +3,17 @@ > * Copyright (c) 2000 Silicon Graphics, Inc. All Rights Reserved. Maybe adding copyright? > */ > +/*\ > + * [Description] > + * > + * Verify that unlink() fails with > + * - ENOENT when file does not exist. > + * - ENOENT when pathname is empty. > + * - ENOENT when a component in pathname does not exist. > + * - EFAULT when pathname points outside the accessible address space. > + * - ENOTDIR when a component used as a directory in pathname is not, > + * in fact, a directory. > + * - ENAMETOOLONG when pathname is too long. I'd remove dot in the end. I don't like 'when', but I don't know anything better. > */ > #include <errno.h> > @@ -39,21 +41,9 @@ static void verify_unlink(unsigned int n) > { > struct test_case_t *tc = &tcases[n]; > - TEST(unlink(tc->name)); > - if (TST_RET != -1) { > - tst_res(TFAIL, "unlink(<%s>) succeeded unexpectedly", > - tc->desc); > - return; > - } > - > - if (TST_ERR == tc->exp_errno) { > - tst_res(TPASS | TTERRNO, "unlink(<%s>) failed as expected", > - tc->desc); > - } else { > - tst_res(TFAIL | TTERRNO, > - "unlink(<%s>) failed, expected errno: %s", > - tc->desc, tst_strerrno(tc->exp_errno)); > - } > + TST_EXP_FAIL(unlink(tc->name), > + tc->exp_errno, > + "%s", tc->desc); This should be on single line. > } > static void setup(void) Other proposed changes: * PATH_MAX is in limits.h + we usually don't put comments why it's included: -#include <sys/param.h> /* for PATH_MAX */ +#include <limits.h> * use size_t in setup static void setup(void) { - unsigned int n; + size_t n; BTW it'd build just with #include "tst_test.h", because all 4 headers are included by headers which tst_test.h includes. I guess most of that it applies to your patch unlink08.c as well, could you please send v2? Kind regards, Petr
Hi Petr, On Thursday, September 1, 2022 12:20:46 PM IST Petr Vorel wrote: > Hi Avinesh, > > > +++ b/testcases/kernel/syscalls/unlink/unlink07.c > > @@ -3,15 +3,17 @@ > > * Copyright (c) 2000 Silicon Graphics, Inc. All Rights Reserved. > Maybe adding copyright? > > */ > > > +/*\ > > + * [Description] > > + * > > + * Verify that unlink() fails with > > + * - ENOENT when file does not exist. > > + * - ENOENT when pathname is empty. > > + * - ENOENT when a component in pathname does not exist. > > + * - EFAULT when pathname points outside the accessible address space. > > + * - ENOTDIR when a component used as a directory in pathname is not, > > + * in fact, a directory. > > + * - ENAMETOOLONG when pathname is too long. > I'd remove dot in the end. > I don't like 'when', but I don't know anything better. > > */ > > > #include <errno.h> > > @@ -39,21 +41,9 @@ static void verify_unlink(unsigned int n) > > { > > struct test_case_t *tc = &tcases[n]; > > > - TEST(unlink(tc->name)); > > - if (TST_RET != -1) { > > - tst_res(TFAIL, "unlink(<%s>) succeeded unexpectedly", > > - tc->desc); > > - return; > > - } > > - > > - if (TST_ERR == tc->exp_errno) { > > - tst_res(TPASS | TTERRNO, "unlink(<%s>) failed as expected", > > - tc->desc); > > - } else { > > - tst_res(TFAIL | TTERRNO, > > - "unlink(<%s>) failed, expected errno: %s", > > - tc->desc, tst_strerrno(tc->exp_errno)); > > - } > > + TST_EXP_FAIL(unlink(tc->name), > > + tc->exp_errno, > > + "%s", tc->desc); > This should be on single line. > > } > > > static void setup(void) > > Other proposed changes: > > * PATH_MAX is in limits.h + we usually don't put comments why it's included: > -#include <sys/param.h> /* for PATH_MAX */ > +#include <limits.h> > > * use size_t in setup > static void setup(void) > { > - unsigned int n; > + size_t n; > > BTW it'd build just with #include "tst_test.h", because all 4 headers are > included by headers which tst_test.h includes. > > I guess most of that it applies to your patch unlink08.c as well, could you > please send v2? Thank you for the review, I have incorporated your suggestions and sent v2. > > Kind regards, > Petr > Regards, Avinesh
diff --git a/testcases/kernel/syscalls/unlink/unlink07.c b/testcases/kernel/syscalls/unlink/unlink07.c index 869bd5f51..8848c1283 100644 --- a/testcases/kernel/syscalls/unlink/unlink07.c +++ b/testcases/kernel/syscalls/unlink/unlink07.c @@ -3,15 +3,17 @@ * Copyright (c) 2000 Silicon Graphics, Inc. All Rights Reserved. */ -/* - * Description: - * The testcase checks the various errnos of the unlink(2). - * 1) unlink() returns ENOENT if file doesn't exist. - * 2) unlink() returns ENOENT if path is empty. - * 3) unlink() returns ENOENT if path contains a non-existent file. - * 4) unlink() returns EFAULT if address is invalid. - * 5) unlink() returns ENOTDIR if path contains a regular file. - * 6) unlink() returns ENAMETOOLONG if path contains a regular file. +/*\ + * [Description] + * + * Verify that unlink() fails with + * - ENOENT when file does not exist. + * - ENOENT when pathname is empty. + * - ENOENT when a component in pathname does not exist. + * - EFAULT when pathname points outside the accessible address space. + * - ENOTDIR when a component used as a directory in pathname is not, + * in fact, a directory. + * - ENAMETOOLONG when pathname is too long. */ #include <errno.h> @@ -39,21 +41,9 @@ static void verify_unlink(unsigned int n) { struct test_case_t *tc = &tcases[n]; - TEST(unlink(tc->name)); - if (TST_RET != -1) { - tst_res(TFAIL, "unlink(<%s>) succeeded unexpectedly", - tc->desc); - return; - } - - if (TST_ERR == tc->exp_errno) { - tst_res(TPASS | TTERRNO, "unlink(<%s>) failed as expected", - tc->desc); - } else { - tst_res(TFAIL | TTERRNO, - "unlink(<%s>) failed, expected errno: %s", - tc->desc, tst_strerrno(tc->exp_errno)); - } + TST_EXP_FAIL(unlink(tc->name), + tc->exp_errno, + "%s", tc->desc); } static void setup(void)
Signed-off-by: Avinesh Kumar <akumar@suse.de> --- testcases/kernel/syscalls/unlink/unlink07.c | 38 ++++++++------------- 1 file changed, 14 insertions(+), 24 deletions(-)