[LTP] [PATCH v1] Port `getxattr01.c` to new test API
Andrea Cervesato
andrea.cervesato@suse.com
Fri Sep 15 16:10:11 CEST 2023
Hi Marius!
On 9/15/23 13:45, Marius Kittler wrote:
> * Utilize `all_filesystems = 1`-mechanism to test on various file
> systems instead of relying on the temporary directory's file system
> to support xattr (which it probably does not as it is commonly a
> tmpfs)
> * Improve error handling
> * Simplify code and description
> * Related issue: https://github.com/linux-test-project/ltp/issues/583
>
> Signed-off-by: Marius Kittler <mkittler@suse.de>
> ---
> .../kernel/syscalls/getxattr/getxattr01.c | 219 ++++++++----------
> 1 file changed, 94 insertions(+), 125 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/getxattr/getxattr01.c b/testcases/kernel/syscalls/getxattr/getxattr01.c
> index cec802a33..e9d5874d9 100644
> --- a/testcases/kernel/syscalls/getxattr/getxattr01.c
> +++ b/testcases/kernel/syscalls/getxattr/getxattr01.c
> @@ -1,38 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> /*
> * 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) 2023 Marius Kittler <mkittler@suse.de>
> */
>
> -/*
> - * Basic tests for getxattr(2) and make sure getxattr(2) handles error
> - * conditions correctly.
> +/*\
> + * [Description]
> *
> - * There are 4 test cases:
> + * Basic tests for getxattr(2), there are 3 test cases:
> * 1. Get an non-existing attribute,
> - * getxattr(2) should return -1 and set errno to ENODATA
> + * getxattr(2) should return -1 and set errno to ENODATA.
> * 2. Buffer size is smaller than attribute value size,
> - * getxattr(2) should return -1 and set errno to ERANGE
> - * 3. Get attribute, getxattr(2) should succeed
> - * 4. Verify the attribute got by getxattr(2) is same as the value we set
> + * getxattr(2) should return -1 and set errno to ERANGE.
> + * 3. getxattr(2) should succeed and return the same value we set
> + * before.
You can use '-' instead of numbers here. That will be automatically seen
as a list by asciidoc.
> */
>
> #include "config.h"
> @@ -47,138 +28,126 @@
We can get rid from most of the includes, since tst_test.h more often
have them already. I would suggest to remove them all and add only what
it's needed for compile.
> #ifdef HAVE_SYS_XATTR_H
> # include <sys/xattr.h>
> #endif
> -#include "test.h"
> -#include "safe_macros.h"
>
> -char *TCID = "getxattr01";
> +#include "tst_test.h"
> +#include "tst_test_macros.h"
>
> -#ifdef HAVE_SYS_XATTR_H
> +#define MNTPOINT "mntpoint"
> #define XATTR_TEST_KEY "user.testkey"
> #define XATTR_TEST_VALUE "this is a test value"
> #define XATTR_TEST_VALUE_SIZE 20
> #define BUFFSIZE 64
>
> -static void setup(void);
> -static void cleanup(void);
> -
> -char filename[BUFSIZ];
> +static char filename[BUFSIZ];
> +static char *workdir;
>
> struct test_case {
> char *fname;
> char *key;
> - char *value;
> + char value[BUFFSIZE];
> size_t size;
> int exp_err;
> -};
> -struct test_case tc[] = {
> - { /* case 00, get non-existing attribute */
> +} tcases[] = {
> + { /* case 00, get non-existing attribute */
> .fname = filename,
> .key = "user.nosuchkey",
> - .value = NULL,
> + .value = {0},
> .size = BUFFSIZE - 1,
> .exp_err = ENODATA,
> - },
> - { /* case 01, small value buffer */
> + },
> + { /* case 01, small value buffer */
> .fname = filename,
> .key = XATTR_TEST_KEY,
> - .value = NULL,
> + .value = {0},
> .size = 1,
> .exp_err = ERANGE,
> - },
> - { /* case 02, get existing attribute */
> + },
> + { /* case 02, get existing attribute */
> .fname = filename,
> .key = XATTR_TEST_KEY,
> - .value = NULL,
> + .value = {0},
> .size = BUFFSIZE - 1,
> .exp_err = 0,
> - },
> + },
> };
>
> -int TST_TOTAL = sizeof(tc) / sizeof(tc[0]) + 1;
> -
> -int main(int argc, char *argv[])
> +static void run(unsigned int i)
> {
> - 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;
> +#ifdef HAVE_SYS_XATTR_H
We usually check for headers only once at the beginning of the file,
just after includes, then use:
#else
TST_TEST_TCONF("System doesn't have <sys/xattr.h>");
#endif
Check fgetxattr01.c source code to get the example.
> + SAFE_CHDIR(workdir);
> +
> + /* create test file and set xattr */
> + struct test_case *tc = &tcases[i];
> + snprintf(tc->fname, BUFSIZ, "getxattr01testfile-%u", i);
> + int fd = SAFE_CREAT(tc->fname, 0644);
> + SAFE_CLOSE(fd);
> + TEST(setxattr(tc->fname, XATTR_TEST_KEY, XATTR_TEST_VALUE,
> + strlen(XATTR_TEST_VALUE), XATTR_CREATE));
Here you can use TST_EXP_PASS instead of TEST
> + if (TST_RET < 0) {
> + if (TST_ERR == ENOTSUP) {
> + tst_res(TCONF, "no xattr support in file system");
> + return;
> }
> -
> - 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");
> + tst_res(TFAIL | TTERRNO, "unexpected setxattr() return code");
> + return;
> }
> -
> - cleanup();
> - tst_exit();
> -}
> -
> -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);
> - 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");
> - }
> + tst_res(TPASS | TTERRNO, "expected setxattr() return code");
> +
> + /* read xattr back */
> + TEST(getxattr(tc->fname, tc->key, tc->value, tc->size));
> + if (TST_ERR == tc->exp_err) {
> + tst_res(TPASS | TTERRNO, "expected getxattr() return code");
> + } else {
> + tst_res(TFAIL | TTERRNO, "unexpected getxattr() return code"
> + " - expected errno %d", tc->exp_err);
> }
Also here you can use TST_EXP_PASS instead of TEST
>
> - /* 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");
> - }
> + /* verify the value for non-error test cases */
> + if (tc->exp_err) {
> + return;
> }
> -
> - TEST_PAUSE;
> + if (TST_RET != XATTR_TEST_VALUE_SIZE) {
Here you can use TST_EXP_EQ_LI
> + tst_res(TFAIL,
> + "getxattr() returned wrong size %ld expected %d",
> + TST_RET, XATTR_TEST_VALUE_SIZE);
> + return;
> + }
> + if (memcmp(tc->value, XATTR_TEST_VALUE, XATTR_TEST_VALUE_SIZE))
> + tst_res(TFAIL, "wrong value, expected \"%s\" got \"%s\"",
> + XATTR_TEST_VALUE, tc->value);
> + else
> + tst_res(TPASS, "right value");
> +#endif
> }
>
> -static void cleanup(void)
> -{
> - tst_rmdir();
> -}
> -#else /* HAVE_SYS_XATTR_H */
> -int main(int argc, char *argv[])
> +static void setup(void)
> {
> - tst_brkm(TCONF, NULL, "<sys/xattr.h> does not exist.");
> +#ifdef HAVE_SYS_XATTR_H
> + char *cwd = SAFE_GETCWD(NULL, 0);
> + workdir = SAFE_MALLOC(strlen(cwd) + strlen(MNTPOINT) + 2);
> + sprintf(workdir, "%s/%s", cwd, MNTPOINT);
> + free(cwd);
Here you can just SAFE_TOUCH the file if you aim to create one.
> +#else
> + tst_brk(TCONF, "<sys/xattr.h> does not exist.");
> +#endif
> }
> +
> +static struct tst_test test = {
> +#ifdef HAVE_SYS_XATTR_H
> + .all_filesystems = 1,
> + .needs_root = 1,
> + .mntpoint = MNTPOINT,
> + .mount_device = 1,
> + .skip_filesystems = (const char *const []) {
> + "exfat",
> + "tmpfs",
> + "ramfs",
> + "nfs",
> + "vfat",
> + NULL
> + },
> #endif
> + .setup = setup,
> + .test = run,
> + .tcnt = ARRAY_SIZE(tcases)
> +};
In your case, to understand LTP API a bit better, I would take
fxgetattr01 as reference, more or less.
Always remember to run "make check-<name of my test>" before sending any
patches, so you can be 100% the code has no text format issues.
Same comments apply to getxattr02 more or less.
Best regards,
Andrea Cervesato
More information about the ltp
mailing list