| Message ID | 20230921090658.11224-1-mkittler@suse.de |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v1] Port getxattr03.c to new test API | expand |
On Thu, Sep 21, 2023 at 11:06:58AM +0200, 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) > * Simplify code and description > > Signed-off-by: Marius Kittler <mkittler@suse.de> > --- > .../kernel/syscalls/getxattr/getxattr03.c | 133 +++++------------- > 1 file changed, 38 insertions(+), 95 deletions(-) > > diff --git a/testcases/kernel/syscalls/getxattr/getxattr03.c b/testcases/kernel/syscalls/getxattr/getxattr03.c > index b6ea14287..78ec79e5c 100644 > --- a/testcases/kernel/syscalls/getxattr/getxattr03.c > +++ b/testcases/kernel/syscalls/getxattr/getxattr03.c > @@ -1,117 +1,60 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > /* > - * Copyright (C) 2012 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) 2012 Red Hat, Inc. > + * Copyright (c) 2023 Marius Kittler <mkittler@suse.de> > */ > > -/* > - * An empty buffer of size zero can be passed into getxattr(2) to return > - * the current size of the named extended attribute. > +/*\ > + * [Description] > + * > + * An empty buffer of size zero can be passed into getxattr(2) to > + * return the current size of the named extended attribute. > */ > > #include "config.h" > -#include <sys/types.h> > -#include <sys/stat.h> > -#include <sys/wait.h> > -#include <errno.h> > -#include <fcntl.h> > -#include <unistd.h> > -#include <signal.h> > -#include <stdio.h> > -#include <stdlib.h> > -#include <string.h> > -#ifdef HAVE_SYS_XATTR_H > -# include <sys/xattr.h> > -#endif > -#include "test.h" > -#include "safe_macros.h" > - > -char *TCID = "getxattr03"; > +#include "tst_test.h" > > #ifdef HAVE_SYS_XATTR_H Normally i saw this kind of ifdef in include/lapi/xattr.h I suppose we need create include/lapi/xattr.h? > +#include <sys/xattr.h> > +#include "tst_safe_macros.h" > + > +#define MNTPOINT "mntpoint" > +#define FNAME MNTPOINT"/getxattr03testfile" > #define XATTR_TEST_KEY "user.testkey" > #define XATTR_TEST_VALUE "test value" > #define XATTR_TEST_VALUE_SIZE (sizeof(XATTR_TEST_VALUE) - 1) > -#define TESTFILE "getxattr03testfile" > - > -static void setup(void); > -static void cleanup(void); > > -int TST_TOTAL = 1; > - > -int main(int argc, char *argv[]) > +static void run(void) > { > - int lc; > - > - tst_parse_opts(argc, argv, NULL, NULL); > - > - setup(); > - > - for (lc = 0; TEST_LOOPING(lc); lc++) { > - tst_count = 0; > - > - TEST(getxattr(TESTFILE, XATTR_TEST_KEY, NULL, 0)); > - > - if (TEST_RETURN == XATTR_TEST_VALUE_SIZE) > - tst_resm(TPASS, "getxattr(2) returned correct value"); > - else > - tst_resm(TFAIL | TTERRNO, "getxattr(2) failed"); > - } > - > - cleanup(); > - tst_exit(); > + TST_EXP_VAL(getxattr(FNAME, XATTR_TEST_KEY, NULL, 0), > + XATTR_TEST_VALUE_SIZE); > } > > static void setup(void) > { > - int fd; > - > - tst_require_root(); > - > - tst_tmpdir(); > - > /* Test for xattr support and set attr value */ > - fd = SAFE_CREAT(cleanup, TESTFILE, 0644); > - close(fd); > - > - if (setxattr(TESTFILE, XATTR_TEST_KEY, XATTR_TEST_VALUE, > - XATTR_TEST_VALUE_SIZE, XATTR_CREATE) == -1) { > - if (errno == ENOTSUP) > - tst_brkm(TCONF, cleanup, "No xattr support in fs or " > - "fs mounted without user_xattr option"); > - else > - tst_brkm(TBROK | TERRNO, cleanup, "setxattr %s failed", > - TESTFILE); > - } > - > - TEST_PAUSE; > + SAFE_TOUCH(FNAME, 0644, NULL); > + SAFE_SETXATTR(FNAME, XATTR_TEST_KEY, XATTR_TEST_VALUE, > + XATTR_TEST_VALUE_SIZE, XATTR_CREATE); > } > > -static void cleanup(void) > -{ > - tst_rmdir(); > -} > +static struct tst_test test = { > + .all_filesystems = 1, > + .needs_root = 1, > + .mntpoint = MNTPOINT, > + .mount_device = 1, > + .skip_filesystems = (const char *const []) { > + "exfat", > + "tmpfs", > + "ramfs", > + "nfs", > + "vfat", > + NULL > + }, > + .setup = setup, > + .test_all = 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("System doesn't have <sys/xattr.h>"); > #endif > -- > 2.42.0 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp
Am Freitag, 22. September 2023, 02:00:32 CEST schrieb Wei Gao: > > #ifdef HAVE_SYS_XATTR_H > > Normally i saw this kind of ifdef in include/lapi/xattr.h > I suppose we need create include/lapi/xattr.h? No, we don't need that header here. This macro is actually defined in `config.h` which also makes sense as it is generated at configuration time. Note that also all builds (including musl) are passing with this change, see https://github.com/Martchus/ltp/actions/runs/6259530698.
On Fri, Sep 22, 2023 at 11:23:47AM +0200, Marius Kittler wrote: > Am Freitag, 22. September 2023, 02:00:32 CEST schrieb Wei Gao: > > > #ifdef HAVE_SYS_XATTR_H > > > > Normally i saw this kind of ifdef in include/lapi/xattr.h > > I suppose we need create include/lapi/xattr.h? > > No, we don't need that header here. This macro is actually defined in `config.h` > which also makes sense as it is generated at configuration time. Note that also > all builds (including musl) are passing with this change, see > https://github.com/Martchus/ltp/actions/runs/6259530698. Maybe some misunderstanding, i mean put ifdef logic into include/lapi/xattr.h instead of define HAVE_SYS_XATTR_H, since i saw ifdef logic for judge exist of xxx.h normally handled in include/lapi/xxx.h. > > > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp
Am Freitag, 22. September 2023, 14:20:38 CEST schrieb Wei Gao: > On Fri, Sep 22, 2023 at 11:23:47AM +0200, Marius Kittler wrote: > > Am Freitag, 22. September 2023, 02:00:32 CEST schrieb Wei Gao: > > > > #ifdef HAVE_SYS_XATTR_H > > > > > > Normally i saw this kind of ifdef in include/lapi/xattr.h > > > I suppose we need create include/lapi/xattr.h? > > > > No, we don't need that header here. This macro is actually defined in > > `config.h` which also makes sense as it is generated at configuration > > time. Note that also all builds (including musl) are passing with this > > change, see > > https://github.com/Martchus/ltp/actions/runs/6259530698. > > Maybe some misunderstanding, i mean put ifdef logic into > include/lapi/xattr.h instead of define HAVE_SYS_XATTR_H, since i saw ifdef > logic for judge exist of xxx.h normally handled in include/lapi/xxx.h. Ah, so I'd add `include/lapi/xattr.h` and to the `#ifdef HAVE_SYS_XATTR_H` there. But is that really the way to go in that situation? I've just checked a few header files in the `include/lapi` directory and there's no precedence for the case when a header is not supported at all and the corresponding test should thus be disabled. These headers seem more for abstracting differences between different (versions) of C libraries but not for handling the case when a test should be skipped completely. Note that the way I wrote this test was suggested to me in "Re: [LTP] [PATCH v1] Port `getxattr01.c` to new test API".
Hello, Marius Kittler <mkittler@suse.de> writes: > Am Freitag, 22. September 2023, 14:20:38 CEST schrieb Wei Gao: >> On Fri, Sep 22, 2023 at 11:23:47AM +0200, Marius Kittler wrote: >> > Am Freitag, 22. September 2023, 02:00:32 CEST schrieb Wei Gao: >> > > > #ifdef HAVE_SYS_XATTR_H >> > > >> > > Normally i saw this kind of ifdef in include/lapi/xattr.h >> > > I suppose we need create include/lapi/xattr.h? >> > >> > No, we don't need that header here. This macro is actually defined in >> > `config.h` which also makes sense as it is generated at configuration >> > time. Note that also all builds (including musl) are passing with this >> > change, see >> > https://github.com/Martchus/ltp/actions/runs/6259530698. >> >> Maybe some misunderstanding, i mean put ifdef logic into >> include/lapi/xattr.h instead of define HAVE_SYS_XATTR_H, since i saw ifdef >> logic for judge exist of xxx.h normally handled in include/lapi/xxx.h. > > Ah, so I'd add `include/lapi/xattr.h` and to the `#ifdef HAVE_SYS_XATTR_H` > there. But is that really the way to go in that situation? I've just checked a > few header files in the `include/lapi` directory and there's no precedence for > the case when a header is not supported at all and the corresponding test > should thus be disabled. These headers seem more for abstracting differences > between different (versions) of C libraries but not for handling the case when > a test should be skipped completely. > > Note that the way I wrote this test was suggested to me in "Re: [LTP] [PATCH > v1] Port `getxattr01.c` to new test API". lib/safe_macros.c included sys/xattr.h without any guards in 2016. I have removed the ifdefs and an inline comment and pushed!
diff --git a/testcases/kernel/syscalls/getxattr/getxattr03.c b/testcases/kernel/syscalls/getxattr/getxattr03.c index b6ea14287..78ec79e5c 100644 --- a/testcases/kernel/syscalls/getxattr/getxattr03.c +++ b/testcases/kernel/syscalls/getxattr/getxattr03.c @@ -1,117 +1,60 @@ +// SPDX-License-Identifier: GPL-2.0-or-later /* - * Copyright (C) 2012 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) 2012 Red Hat, Inc. + * Copyright (c) 2023 Marius Kittler <mkittler@suse.de> */ -/* - * An empty buffer of size zero can be passed into getxattr(2) to return - * the current size of the named extended attribute. +/*\ + * [Description] + * + * An empty buffer of size zero can be passed into getxattr(2) to + * return the current size of the named extended attribute. */ #include "config.h" -#include <sys/types.h> -#include <sys/stat.h> -#include <sys/wait.h> -#include <errno.h> -#include <fcntl.h> -#include <unistd.h> -#include <signal.h> -#include <stdio.h> -#include <stdlib.h> -#include <string.h> -#ifdef HAVE_SYS_XATTR_H -# include <sys/xattr.h> -#endif -#include "test.h" -#include "safe_macros.h" - -char *TCID = "getxattr03"; +#include "tst_test.h" #ifdef HAVE_SYS_XATTR_H +#include <sys/xattr.h> +#include "tst_safe_macros.h" + +#define MNTPOINT "mntpoint" +#define FNAME MNTPOINT"/getxattr03testfile" #define XATTR_TEST_KEY "user.testkey" #define XATTR_TEST_VALUE "test value" #define XATTR_TEST_VALUE_SIZE (sizeof(XATTR_TEST_VALUE) - 1) -#define TESTFILE "getxattr03testfile" - -static void setup(void); -static void cleanup(void); -int TST_TOTAL = 1; - -int main(int argc, char *argv[]) +static void run(void) { - int lc; - - tst_parse_opts(argc, argv, NULL, NULL); - - setup(); - - for (lc = 0; TEST_LOOPING(lc); lc++) { - tst_count = 0; - - TEST(getxattr(TESTFILE, XATTR_TEST_KEY, NULL, 0)); - - if (TEST_RETURN == XATTR_TEST_VALUE_SIZE) - tst_resm(TPASS, "getxattr(2) returned correct value"); - else - tst_resm(TFAIL | TTERRNO, "getxattr(2) failed"); - } - - cleanup(); - tst_exit(); + TST_EXP_VAL(getxattr(FNAME, XATTR_TEST_KEY, NULL, 0), + XATTR_TEST_VALUE_SIZE); } static void setup(void) { - int fd; - - tst_require_root(); - - tst_tmpdir(); - /* Test for xattr support and set attr value */ - fd = SAFE_CREAT(cleanup, TESTFILE, 0644); - close(fd); - - if (setxattr(TESTFILE, XATTR_TEST_KEY, XATTR_TEST_VALUE, - XATTR_TEST_VALUE_SIZE, XATTR_CREATE) == -1) { - if (errno == ENOTSUP) - tst_brkm(TCONF, cleanup, "No xattr support in fs or " - "fs mounted without user_xattr option"); - else - tst_brkm(TBROK | TERRNO, cleanup, "setxattr %s failed", - TESTFILE); - } - - TEST_PAUSE; + SAFE_TOUCH(FNAME, 0644, NULL); + SAFE_SETXATTR(FNAME, XATTR_TEST_KEY, XATTR_TEST_VALUE, + XATTR_TEST_VALUE_SIZE, XATTR_CREATE); } -static void cleanup(void) -{ - tst_rmdir(); -} +static struct tst_test test = { + .all_filesystems = 1, + .needs_root = 1, + .mntpoint = MNTPOINT, + .mount_device = 1, + .skip_filesystems = (const char *const []) { + "exfat", + "tmpfs", + "ramfs", + "nfs", + "vfat", + NULL + }, + .setup = setup, + .test_all = 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("System doesn't have <sys/xattr.h>"); #endif
* 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) * Simplify code and description Signed-off-by: Marius Kittler <mkittler@suse.de> --- .../kernel/syscalls/getxattr/getxattr03.c | 133 +++++------------- 1 file changed, 38 insertions(+), 95 deletions(-)