Message ID | 20240121161159.4106646-2-ruansy.fnst@fujitsu.com |
---|---|
State | Changes Requested |
Headers | show |
Series | [1/2] getxattr04,05: Add [Description] tag | expand |
Hi Shiyang, some comments below. On Sunday, January 21, 2024 5:11:59 PM CET Shiyang Ruan wrote: > Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com> > --- > .../kernel/syscalls/getxattr/getxattr01.c | 188 ++++++------------ > 1 file changed, 61 insertions(+), 127 deletions(-) > > diff --git a/testcases/kernel/syscalls/getxattr/getxattr01.c > b/testcases/kernel/syscalls/getxattr/getxattr01.c index > cec802a33..e11f00d46 100644 > --- a/testcases/kernel/syscalls/getxattr/getxattr01.c > +++ b/testcases/kernel/syscalls/getxattr/getxattr01.c > @@ -1,28 +1,12 @@ > +// SPDX-License-Identifier: GPL-2.0-only > /* > * Copyright (C) 2011 Red Hat, Inc. > - * > - * This program is free software; you can redistribute it and/or > - * modify it under the terms of version 2 of the GNU General Public > - * License as published by the Free Software Foundation. > - * > - * This program is distributed in the hope that it would be useful, > - * but WITHOUT ANY WARRANTY; without even the implied warranty of > - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > - * > - * Further, this software is distributed without any warranty that it > - * is free of the rightful claim of any third person regarding > - * infringement or the like. Any license provided herein, whether > - * implied or otherwise, applies only to this software file. Patent > - * licenses, if any, provided herein do not apply to combinations of > - * this program with other software, or any other product whatsoever. > - * > - * You should have received a copy of the GNU General Public License > - * along with this program; if not, write the Free Software > - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA > - * 02110-1301, USA. > + * Copyright (c) Linux Test Project, 2012-2024 > */ > > -/* > +/*\ > + * [Description] > + * > * Basic tests for getxattr(2) and make sure getxattr(2) handles error > * conditions correctly. > * > @@ -35,22 +19,11 @@ > * 4. Verify the attribute got by getxattr(2) is same as the value we set > */ > > -#include "config.h" > -#include <sys/types.h> > -#include <sys/stat.h> > -#include <errno.h> > -#include <fcntl.h> > -#include <unistd.h> > -#include <stdio.h> > #include <stdlib.h> > -#include <string.h> > +#include "tst_test.h" > #ifdef HAVE_SYS_XATTR_H > # include <sys/xattr.h> > #endif > -#include "test.h" > -#include "safe_macros.h" > - > -char *TCID = "getxattr01"; > > #ifdef HAVE_SYS_XATTR_H > #define XATTR_TEST_KEY "user.testkey" > @@ -58,85 +31,51 @@ char *TCID = "getxattr01"; > #define XATTR_TEST_VALUE_SIZE 20 > #define BUFFSIZE 64 > > -static void setup(void); > -static void cleanup(void); > - > -char filename[BUFSIZ]; > +static char filename[BUFSIZ]; > > -struct test_case { > +static struct tcase { > char *fname; > char *key; > char *value; > size_t size; > int exp_err; > -}; > -struct test_case tc[] = { > - { /* case 00, get non-existing attribute */ > - .fname = filename, > - .key = "user.nosuchkey", > - .value = NULL, > - .size = BUFFSIZE - 1, > - .exp_err = ENODATA, > - }, > - { /* case 01, small value buffer */ > - .fname = filename, > - .key = XATTR_TEST_KEY, > - .value = NULL, > - .size = 1, > - .exp_err = ERANGE, > - }, > - { /* case 02, get existing attribute */ > - .fname = filename, > - .key = XATTR_TEST_KEY, > - .value = NULL, > - .size = BUFFSIZE - 1, > - .exp_err = 0, > - }, > +} tcases[] = { > + /* case 00, get non-existing attribute */ Can we please remove these comments, as we are describing the testcases in the description already. > + { filename, "user.nosuchkey", NULL, BUFFSIZE - 1, ENODATA }, > + /* case 01, small value buffer */ > + { filename, XATTR_TEST_KEY, NULL, 1, ERANGE }, > + /* case 02, get existing attribute */ > + { filename, XATTR_TEST_KEY, NULL, BUFFSIZE - 1, 0 }, > }; > > -int TST_TOTAL = sizeof(tc) / sizeof(tc[0]) + 1; > - > -int main(int argc, char *argv[]) > +static void run(unsigned int n) > { > - int lc; > - int i; > - > - tst_parse_opts(argc, argv, NULL, NULL); > - > - setup(); > - > - for (lc = 0; TEST_LOOPING(lc); lc++) { > - tst_count = 0; > - > - for (i = 0; i < (int)ARRAY_SIZE(tc); i++) { > - TEST(getxattr(tc[i].fname, tc[i].key, tc[i].value, > - tc[i].size)); > - > - if (TEST_ERRNO == tc[i].exp_err) { > - tst_resm(TPASS | TTERRNO, "expected behavior"); > - } else { > - tst_resm(TFAIL | TTERRNO, "unexpected behavior" > - "- expected errno %d - Got", > - tc[i].exp_err); > - } > - } > - > - if (TEST_RETURN != XATTR_TEST_VALUE_SIZE) { > - tst_resm(TFAIL, > - "getxattr() returned wrong size %ld expected %d", > - TEST_RETURN, XATTR_TEST_VALUE_SIZE); > - continue; > - } > - > - if (memcmp(tc[i - 1].value, XATTR_TEST_VALUE, XATTR_TEST_VALUE_SIZE)) > - tst_resm(TFAIL, "Wrong value, expect \"%s\" got \"%s\"", > - XATTR_TEST_VALUE, tc[i - 1].value); > - else > - tst_resm(TPASS, "Got the right value"); > + struct tcase *tc = &tcases[n]; > + > + TEST(getxattr(tc->fname, tc->key, tc->value, tc->size)); > + if (TST_ERR == tc->exp_err) { > + tst_res(TPASS | TTERRNO, "expected behavior"); > + } else { > + tst_res(TFAIL | TTERRNO, "unexpected behavior" > + "- expected errno %d - Got", > + tc->exp_err); > } > > - cleanup(); > - tst_exit(); > + /* The last check: > + * Verify the attribute got by getxattr(2) is same as the value we set > + */ > + if (n == ARRAY_SIZE(tcases) - 1) { Though ideally we separate the errno checking testcases and syscall success tests in different test files. But here we can simplify this using TST_EXP_* macros, maybe something like - if (tc->exp_err == 0) { TST_EXP_VAL(getxattr(tc->fname, tc->key, tc->value, tc->size), XATTR_TEST_VALUE_SIZE); if (memcmp(tc->value, XATTR_TEST_VALUE, XATTR_TEST_VALUE_SIZE)) tst_res(TFAIL, "Wrong value, expect \"%s\" got \"%s\"", XATTR_TEST_VALUE, tc->value); else tst_res(TPASS, "getxattr() retrieved expected value"); } else { TST_EXP_FAIL(getxattr(tc->fname, tc->key, tc->value, tc->size), tc->exp_err); } > + if (TST_RET != XATTR_TEST_VALUE_SIZE) > + tst_res(TFAIL, > + "getxattr() returned wrong size %ld expected %d", > + TST_RET, XATTR_TEST_VALUE_SIZE); > + > + if (memcmp(tc->value, XATTR_TEST_VALUE, XATTR_TEST_VALUE_SIZE)) > + tst_res(TFAIL, "Wrong value, expect \"%s\" got \"%s\"", > + XATTR_TEST_VALUE, tc->value); > + else > + tst_res(TPASS, "Got the right value"); > + } > } > > static void setup(void) > @@ -144,41 +83,36 @@ static void setup(void) > int fd; > unsigned int i; > > - tst_require_root(); > - > - tst_tmpdir(); > - > /* Create test file and setup initial xattr */ > snprintf(filename, BUFSIZ, "getxattr01testfile"); > - fd = SAFE_CREAT(cleanup, filename, 0644); > + fd = SAFE_CREAT(filename, 0644); > close(fd); we can use SAFE_CLOSE() > - if (setxattr(filename, XATTR_TEST_KEY, XATTR_TEST_VALUE, > - strlen(XATTR_TEST_VALUE), XATTR_CREATE) == -1) { > - if (errno == ENOTSUP) { > - tst_brkm(TCONF, cleanup, "No xattr support in fs or " > - "mount without user_xattr option"); > - } > - } > > - /* Prepare test cases */ > - for (i = 0; i < ARRAY_SIZE(tc); i++) { > - tc[i].value = malloc(BUFFSIZE); > - if (tc[i].value == NULL) { > - tst_brkm(TBROK | TERRNO, cleanup, > - "Cannot allocate memory"); > - } > - } > + SAFE_SETXATTR(filename, XATTR_TEST_KEY, XATTR_TEST_VALUE, > + strlen(XATTR_TEST_VALUE), XATTR_CREATE); > > - TEST_PAUSE; > + for (i = 0; i < ARRAY_SIZE(tcases); i++) > + tcases[i].value = SAFE_MALLOC(BUFFSIZE); > } > > static void cleanup(void) > { > - tst_rmdir(); > + unsigned int i; > + > + for (i = 0; i < ARRAY_SIZE(tcases); i++) > + if (tcases[i].value != NULL) > + free(tcases[i].value); > } > + > +static struct tst_test test = { > + .needs_tmpdir = 1, > + .needs_root = 1, > + .setup = setup, > + .cleanup = cleanup, > + .tcnt = ARRAY_SIZE(tcases), > + .test = run, > +}; > + > #else /* HAVE_SYS_XATTR_H */ > -int main(int argc, char *argv[]) > -{ > - tst_brkm(TCONF, NULL, "<sys/xattr.h> does not exist."); > -} > + TST_TEST_TCONF("<sys/xattr.h> does not exist."); > #endif Regards, Avinesh
diff --git a/testcases/kernel/syscalls/getxattr/getxattr01.c b/testcases/kernel/syscalls/getxattr/getxattr01.c index cec802a33..e11f00d46 100644 --- a/testcases/kernel/syscalls/getxattr/getxattr01.c +++ b/testcases/kernel/syscalls/getxattr/getxattr01.c @@ -1,28 +1,12 @@ +// SPDX-License-Identifier: GPL-2.0-only /* * Copyright (C) 2011 Red Hat, Inc. - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of version 2 of the GNU General Public - * License as published by the Free Software Foundation. - * - * This program is distributed in the hope that it would be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. - * - * Further, this software is distributed without any warranty that it - * is free of the rightful claim of any third person regarding - * infringement or the like. Any license provided herein, whether - * implied or otherwise, applies only to this software file. Patent - * licenses, if any, provided herein do not apply to combinations of - * this program with other software, or any other product whatsoever. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA - * 02110-1301, USA. + * Copyright (c) Linux Test Project, 2012-2024 */ -/* +/*\ + * [Description] + * * Basic tests for getxattr(2) and make sure getxattr(2) handles error * conditions correctly. * @@ -35,22 +19,11 @@ * 4. Verify the attribute got by getxattr(2) is same as the value we set */ -#include "config.h" -#include <sys/types.h> -#include <sys/stat.h> -#include <errno.h> -#include <fcntl.h> -#include <unistd.h> -#include <stdio.h> #include <stdlib.h> -#include <string.h> +#include "tst_test.h" #ifdef HAVE_SYS_XATTR_H # include <sys/xattr.h> #endif -#include "test.h" -#include "safe_macros.h" - -char *TCID = "getxattr01"; #ifdef HAVE_SYS_XATTR_H #define XATTR_TEST_KEY "user.testkey" @@ -58,85 +31,51 @@ char *TCID = "getxattr01"; #define XATTR_TEST_VALUE_SIZE 20 #define BUFFSIZE 64 -static void setup(void); -static void cleanup(void); - -char filename[BUFSIZ]; +static char filename[BUFSIZ]; -struct test_case { +static struct tcase { char *fname; char *key; char *value; size_t size; int exp_err; -}; -struct test_case tc[] = { - { /* case 00, get non-existing attribute */ - .fname = filename, - .key = "user.nosuchkey", - .value = NULL, - .size = BUFFSIZE - 1, - .exp_err = ENODATA, - }, - { /* case 01, small value buffer */ - .fname = filename, - .key = XATTR_TEST_KEY, - .value = NULL, - .size = 1, - .exp_err = ERANGE, - }, - { /* case 02, get existing attribute */ - .fname = filename, - .key = XATTR_TEST_KEY, - .value = NULL, - .size = BUFFSIZE - 1, - .exp_err = 0, - }, +} tcases[] = { + /* case 00, get non-existing attribute */ + { filename, "user.nosuchkey", NULL, BUFFSIZE - 1, ENODATA }, + /* case 01, small value buffer */ + { filename, XATTR_TEST_KEY, NULL, 1, ERANGE }, + /* case 02, get existing attribute */ + { filename, XATTR_TEST_KEY, NULL, BUFFSIZE - 1, 0 }, }; -int TST_TOTAL = sizeof(tc) / sizeof(tc[0]) + 1; - -int main(int argc, char *argv[]) +static void run(unsigned int n) { - int lc; - int i; - - tst_parse_opts(argc, argv, NULL, NULL); - - setup(); - - for (lc = 0; TEST_LOOPING(lc); lc++) { - tst_count = 0; - - for (i = 0; i < (int)ARRAY_SIZE(tc); i++) { - TEST(getxattr(tc[i].fname, tc[i].key, tc[i].value, - tc[i].size)); - - if (TEST_ERRNO == tc[i].exp_err) { - tst_resm(TPASS | TTERRNO, "expected behavior"); - } else { - tst_resm(TFAIL | TTERRNO, "unexpected behavior" - "- expected errno %d - Got", - tc[i].exp_err); - } - } - - if (TEST_RETURN != XATTR_TEST_VALUE_SIZE) { - tst_resm(TFAIL, - "getxattr() returned wrong size %ld expected %d", - TEST_RETURN, XATTR_TEST_VALUE_SIZE); - continue; - } - - if (memcmp(tc[i - 1].value, XATTR_TEST_VALUE, XATTR_TEST_VALUE_SIZE)) - tst_resm(TFAIL, "Wrong value, expect \"%s\" got \"%s\"", - XATTR_TEST_VALUE, tc[i - 1].value); - else - tst_resm(TPASS, "Got the right value"); + struct tcase *tc = &tcases[n]; + + TEST(getxattr(tc->fname, tc->key, tc->value, tc->size)); + if (TST_ERR == tc->exp_err) { + tst_res(TPASS | TTERRNO, "expected behavior"); + } else { + tst_res(TFAIL | TTERRNO, "unexpected behavior" + "- expected errno %d - Got", + tc->exp_err); } - cleanup(); - tst_exit(); + /* The last check: + * Verify the attribute got by getxattr(2) is same as the value we set + */ + if (n == ARRAY_SIZE(tcases) - 1) { + if (TST_RET != XATTR_TEST_VALUE_SIZE) + tst_res(TFAIL, + "getxattr() returned wrong size %ld expected %d", + TST_RET, XATTR_TEST_VALUE_SIZE); + + if (memcmp(tc->value, XATTR_TEST_VALUE, XATTR_TEST_VALUE_SIZE)) + tst_res(TFAIL, "Wrong value, expect \"%s\" got \"%s\"", + XATTR_TEST_VALUE, tc->value); + else + tst_res(TPASS, "Got the right value"); + } } static void setup(void) @@ -144,41 +83,36 @@ static void setup(void) int fd; unsigned int i; - tst_require_root(); - - tst_tmpdir(); - /* Create test file and setup initial xattr */ snprintf(filename, BUFSIZ, "getxattr01testfile"); - fd = SAFE_CREAT(cleanup, filename, 0644); + fd = SAFE_CREAT(filename, 0644); close(fd); - if (setxattr(filename, XATTR_TEST_KEY, XATTR_TEST_VALUE, - strlen(XATTR_TEST_VALUE), XATTR_CREATE) == -1) { - if (errno == ENOTSUP) { - tst_brkm(TCONF, cleanup, "No xattr support in fs or " - "mount without user_xattr option"); - } - } - /* Prepare test cases */ - for (i = 0; i < ARRAY_SIZE(tc); i++) { - tc[i].value = malloc(BUFFSIZE); - if (tc[i].value == NULL) { - tst_brkm(TBROK | TERRNO, cleanup, - "Cannot allocate memory"); - } - } + SAFE_SETXATTR(filename, XATTR_TEST_KEY, XATTR_TEST_VALUE, + strlen(XATTR_TEST_VALUE), XATTR_CREATE); - TEST_PAUSE; + for (i = 0; i < ARRAY_SIZE(tcases); i++) + tcases[i].value = SAFE_MALLOC(BUFFSIZE); } static void cleanup(void) { - tst_rmdir(); + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(tcases); i++) + if (tcases[i].value != NULL) + free(tcases[i].value); } + +static struct tst_test test = { + .needs_tmpdir = 1, + .needs_root = 1, + .setup = setup, + .cleanup = cleanup, + .tcnt = ARRAY_SIZE(tcases), + .test = run, +}; + #else /* HAVE_SYS_XATTR_H */ -int main(int argc, char *argv[]) -{ - tst_brkm(TCONF, NULL, "<sys/xattr.h> does not exist."); -} + TST_TEST_TCONF("<sys/xattr.h> does not exist."); #endif
Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com> --- .../kernel/syscalls/getxattr/getxattr01.c | 188 ++++++------------ 1 file changed, 61 insertions(+), 127 deletions(-)