| Message ID | 20220708170634.842-1-akumar@suse.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fstat02.c: simplify using TST_EXP_*() macros | expand |
Hi Avinesh, LGTM, thanks! Minor comments below. Reviewed-by: Petr Vorel <pvorel@suse.cz> ... > +++ b/testcases/kernel/syscalls/fstat/fstat02.c ... > -#include <errno.h> > -#include <unistd.h> > -#include <sys/stat.h> > -#include <sys/types.h> > #include "tst_test.h" > -#include "tst_safe_macros.h" nit: obviously works without all these headers, but <sys/stat.h> is needed (fstat) but included by tst_test.h > + > +/*\ [Description] > + * This is wrong, causes docparse not being formatted. It must be: /*\ * [Description] (can be fixed before merge) > * Tests if fstat() returns correctly and reports correct file information > * using the stat structure. > */ ... > - tst_res(TPASS, "fstat() reported correct values."); > + TST_EXP_PASS(fstat(fildes, &stat_buf)); > + TST_EXP_EQ_LI(stat_buf.st_uid, user_id); > + TST_EXP_EQ_LI(stat_buf.st_gid, group_id); nit: At least st_uid and st_gid are unsigned, thus maybe TST_EXP_EQ_LU? > + TST_EXP_EQ_LI(stat_buf.st_size, FILE_SIZE); > + TST_EXP_EQ_LI(stat_buf.st_mode & 0777, FILE_MODE); > + TST_EXP_EQ_LI(stat_buf.st_nlink, NLINK); > } Kind regards, Petr
On Monday, July 11, 2022 9:21:56 PM IST Petr Vorel wrote: > Hi Avinesh, > > > LGTM, thanks! > Minor comments below. Hi Petr, Thank you! I agree with your comments here. Do I need to send v2 or you will merge with these changes? > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > ... > > +++ b/testcases/kernel/syscalls/fstat/fstat02.c > ... > > -#include <errno.h> > > -#include <unistd.h> > > -#include <sys/stat.h> > > -#include <sys/types.h> > > #include "tst_test.h" > > -#include "tst_safe_macros.h" > > nit: obviously works without all these headers, but <sys/stat.h> is > needed (fstat) but included by tst_test.h > > > + > > +/*\ [Description] > > + * > > This is wrong, causes docparse not being formatted. It must be: > /*\ > * [Description] > > (can be fixed before merge) > > > * Tests if fstat() returns correctly and reports correct file information > > * using the stat structure. > > */ > ... > > - tst_res(TPASS, "fstat() reported correct values."); > > + TST_EXP_PASS(fstat(fildes, &stat_buf)); > > + TST_EXP_EQ_LI(stat_buf.st_uid, user_id); > > + TST_EXP_EQ_LI(stat_buf.st_gid, group_id); > nit: At least st_uid and st_gid are unsigned, thus maybe TST_EXP_EQ_LU? > > + TST_EXP_EQ_LI(stat_buf.st_size, FILE_SIZE); > > + TST_EXP_EQ_LI(stat_buf.st_mode & 0777, FILE_MODE); > > + TST_EXP_EQ_LI(stat_buf.st_nlink, NLINK); > > } > > Kind regards, > Petr > -- Avinesh
> On Monday, July 11, 2022 9:21:56 PM IST Petr Vorel wrote: > > Hi Avinesh, > > LGTM, thanks! > > Minor comments below. > Hi Petr, Thank you! I agree with your comments here. > Do I need to send v2 or you will merge with these changes? I'm sorry, I overlooked your question (vacation). Going to merge v2. Kind regards, Petr
diff --git a/testcases/kernel/syscalls/fstat/fstat02.c b/testcases/kernel/syscalls/fstat/fstat02.c index 2f9632edf..2142ae3e6 100644 --- a/testcases/kernel/syscalls/fstat/fstat02.c +++ b/testcases/kernel/syscalls/fstat/fstat02.c @@ -4,17 +4,14 @@ * 07/2001 Ported by Wayne Boyer * 05/2019 Ported to new library: Christian Amann <camann@suse.com> */ -/* + +/*\ [Description] + * * Tests if fstat() returns correctly and reports correct file information * using the stat structure. */ -#include <errno.h> -#include <unistd.h> -#include <sys/stat.h> -#include <sys/types.h> #include "tst_test.h" -#include "tst_safe_macros.h" #define TESTFILE "test_file" #define LINK_TESTFILE "link_test_file" @@ -29,50 +26,12 @@ static int fildes; static void run(void) { - int fail = 0; - - TEST(fstat(fildes, &stat_buf)); - - if (TST_RET != 0) { - tst_res(TFAIL | TTERRNO, "fstat() failed"); - return; - } - - fail = 0; - if (stat_buf.st_uid != user_id) { - tst_res(TFAIL, "stat_buf.st_uid = %i expected %i", - stat_buf.st_uid, user_id); - fail++; - } - - if (stat_buf.st_gid != group_id) { - tst_res(TFAIL, "stat_buf.st_gid = %i expected %i", - stat_buf.st_gid, group_id); - fail++; - } - - if (stat_buf.st_size != FILE_SIZE) { - tst_res(TFAIL, "stat_buf.st_size = %li expected %i", - (long)stat_buf.st_size, FILE_SIZE); - fail++; - } - - if ((stat_buf.st_mode & 0777) != FILE_MODE) { - tst_res(TFAIL, "stat_buf.st_mode = %o expected %o", - (stat_buf.st_mode & 0777), FILE_MODE); - fail++; - } - - if (stat_buf.st_nlink != NLINK) { - tst_res(TFAIL, "stat_buf.st_nlink = %li expected %i", - (long)stat_buf.st_nlink, NLINK); - fail++; - } - - if (fail) - return; - - tst_res(TPASS, "fstat() reported correct values."); + TST_EXP_PASS(fstat(fildes, &stat_buf)); + TST_EXP_EQ_LI(stat_buf.st_uid, user_id); + TST_EXP_EQ_LI(stat_buf.st_gid, group_id); + TST_EXP_EQ_LI(stat_buf.st_size, FILE_SIZE); + TST_EXP_EQ_LI(stat_buf.st_mode & 0777, FILE_MODE); + TST_EXP_EQ_LI(stat_buf.st_nlink, NLINK); } static void setup(void)
Simplifying the tests using TST_EXP_*() macros and removing the unnecessary header includes Signed-off-by: Avinesh Kumar <akumar@suse.de> --- testcases/kernel/syscalls/fstat/fstat02.c | 59 ++++------------------- 1 file changed, 9 insertions(+), 50 deletions(-)