[LTP] [PATCH v1] Port getxattr03.c to new test API

[LTP] [PATCH v1] Port getxattr03.c to new test API

[Marius Kittler]

* 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
+#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

Re: [LTP] [PATCH v1] Port getxattr03.c to new test API

[Wei Gao via ltp]

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

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] Port getxattr03.c to new test API

[Marius Kittler]

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.




-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] Port getxattr03.c to new test API

[Wei Gao via ltp]

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

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] Port getxattr03.c to new test API

[Marius Kittler]

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".



-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] Port getxattr03.c to new test API

[Richard Palethorpe]

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!

-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp