[LTP] [PATCH v4] Port getxattr02.c to new test API

[LTP] [PATCH v4] Port getxattr02.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)
* Improve error handling
* Simplify code and description

Signed-off-by: Marius Kittler <mkittler@suse.de>
---
 .../kernel/syscalls/getxattr/getxattr02.c     | 192 +++++++-----------
 1 file changed, 73 insertions(+), 119 deletions(-)

diff --git a/testcases/kernel/syscalls/getxattr/getxattr02.c b/testcases/kernel/syscalls/getxattr/getxattr02.c
index a42057d0a..dbbce8f16 100644
--- a/testcases/kernel/syscalls/getxattr/getxattr02.c
+++ b/testcases/kernel/syscalls/getxattr/getxattr02.c
@@ -1,64 +1,42 @@
+// 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) 2011  Red Hat, Inc.
+ * Copyright (c) 2023 Marius Kittler <mkittler@suse.de>
  */
 
-/*
+/*\
+ * [Description]
+ *
  * In the user.* namespace, only regular files and directories can
  * have extended attributes. Otherwise getxattr(2) will return -1
  * and set errno to ENODATA.
  *
  * There are 4 test cases:
- * 1. Get attribute from a FIFO, setxattr(2) should return -1 and
+ * - Get attribute from a FIFO, setxattr(2) should return -1 and
  *    set errno to ENODATA
- * 2. Get attribute from a char special file, setxattr(2) should
+ * - Get attribute from a char special file, setxattr(2) should
  *    return -1 and set errno to ENODATA
- * 3. Get attribute from a block special file, setxattr(2) should
+ * - Get attribute from a block special file, setxattr(2) should
  *    return -1 and set errno to ENODATA
- * 4. Get attribute from a UNIX domain socket, setxattr(2) should
+ * - Get attribute from a UNIX domain socket, setxattr(2) should
  *    return -1 and set errno to ENODATA
  */
 
 #include "config.h"
 #include <sys/types.h>
-#include <sys/stat.h>
 #include <sys/sysmacros.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 = "getxattr02";
+#include "tst_test.h"
 
 #ifdef HAVE_SYS_XATTR_H
+#include <sys/xattr.h>
+
+#include "tst_test_macros.h"
+
+#define MNTPOINT "mntpoint"
+#define FNAME MNTPOINT"/getxattr02"
 #define XATTR_TEST_KEY "user.testkey"
 
 #define FIFO "getxattr02fifo"
@@ -66,94 +44,70 @@ char *TCID = "getxattr02";
 #define BLK  "getxattr02blk"
 #define SOCK "getxattr02sock"
 
-static void setup(void);
-static void cleanup(void);
-
-static char *tc[] = {
-	FIFO,			/* case 00, get attr from fifo */
-	CHR,			/* case 01, get attr from char special */
-	BLK,			/* case 02, get attr from block special */
-	SOCK,			/* case 03, get attr from UNIX domain socket */
+static struct test_case {
+	char *fname;
+	int mode;
+} tcases[] = {
+	{ /* case 00, get attr from fifo */
+	 .fname = FNAME FIFO,
+	 .mode = S_IFIFO,
+	},
+	{ /* case 01, get attr from char special */
+	 .fname = FNAME CHR,
+	 .mode = S_IFCHR,
+	},
+	{ /* case 02, get attr from block special */
+	 .fname = FNAME BLK,
+	 .mode = S_IFBLK,
+	},
+	{ /* case 03, get attr from UNIX domain socket */
+	 .fname = FNAME SOCK,
+	 .mode = S_IFSOCK,
+	},
 };
 
-int TST_TOTAL = sizeof(tc) / sizeof(tc[0]);
-
-int main(int argc, char *argv[])
+static void run(unsigned int i)
 {
-	int lc;
-	int i;
-	int exp_eno;
 	char buf[BUFSIZ];
-
-	tst_parse_opts(argc, argv, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-		exp_eno = ENODATA;
-
-		for (i = 0; i < TST_TOTAL; i++) {
-			TEST(getxattr(tc[0], XATTR_TEST_KEY, buf, BUFSIZ));
-
-			if (TEST_RETURN == -1 && TEST_ERRNO == exp_eno)
-				tst_resm(TPASS | TTERRNO, "expected behavior");
-			else
-				tst_resm(TFAIL | TTERRNO, "unexpected behavior"
-					 " - expected errno %d - Got", exp_eno);
-		}
-	}
-
-	cleanup();
-	tst_exit();
+	struct test_case *tc = &tcases[i];
+	dev_t dev = tc->mode == S_IFCHR ? makedev(1, 3) : 0u;
+
+	if (mknod(tc->fname, tc->mode | 0777, dev) < 0)
+		tst_brk(TBROK | TERRNO, "create %s (mode %i) failed", tc->fname, tc->mode);
+
+	TEST(getxattr(tc->fname, XATTR_TEST_KEY, buf, BUFSIZ));
+	if (TST_RET == -1 && TST_ERR == ENODATA)
+		tst_res(TPASS | TTERRNO, "expected return value");
+	else
+		tst_res(TFAIL | TTERRNO, "unexpected return value"
+				" - expected errno %d - got", ENODATA);
 }
 
 static void setup(void)
 {
-	int fd;
-	dev_t dev;
-
-	tst_require_root();
-
-	tst_tmpdir();
-
-	/* Test for xattr support */
-	fd = SAFE_CREAT(cleanup, "testfile", 0644);
-	close(fd);
-	if (setxattr("testfile", "user.test", "test", 4, XATTR_CREATE) == -1)
-		if (errno == ENOTSUP)
-			tst_brkm(TCONF, cleanup, "No xattr support in fs or "
-				 "mount without user_xattr option");
-	unlink("testfile");
-
-	/* Create test files */
-	if (mknod(FIFO, S_IFIFO | 0777, 0) == -1)
-		tst_brkm(TBROK | TERRNO, cleanup, "Create FIFO(%s) failed",
-			 FIFO);
-
-	dev = makedev(1, 3);
-	if (mknod(CHR, S_IFCHR | 0777, dev) == -1)
-		tst_brkm(TBROK | TERRNO, cleanup, "Create char special(%s)"
-			 " failed", CHR);
-
-	if (mknod(BLK, S_IFBLK | 0777, 0) == -1)
-		tst_brkm(TBROK | TERRNO, cleanup, "Create block special(%s)"
-			 " failed", BLK);
-
-	if (mknod(SOCK, S_IFSOCK | 0777, 0) == -1)
-		tst_brkm(TBROK | TERRNO, cleanup, "Create socket(%s) failed",
-			 SOCK);
-
-	TEST_PAUSE;
+	/* assert xattr support in the current filesystem */
+	SAFE_TOUCH(FNAME, 0644, NULL);
+	SAFE_SETXATTR(FNAME, "user.test", "test", 4, XATTR_CREATE);
 }
 
-static void cleanup(void)
-{
-	tst_rmdir();
-}
-#else /* HAVE_SYS_XATTR_H */
-int main(int argc, char *argv[])
-{
-	tst_brkm(TCONF, NULL, "<sys/xattr.h> does not exist.");
-}
+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 = run,
+	.tcnt = ARRAY_SIZE(tcases)
+};
+
+#else
+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 v4] Port getxattr02.c to new test API

[Petr Vorel]

Hi Marius,

Please, could you remove HAVE_SYS_XATTR_H check and TST_TEST_TCONF() ?
<sys/xattr.h> has been around for a long time?

I'll send a patch which does it on other files and from configure.

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

> Signed-off-by: Marius Kittler <mkittler@suse.de>
> ---
>  .../kernel/syscalls/getxattr/getxattr02.c     | 192 +++++++-----------
>  1 file changed, 73 insertions(+), 119 deletions(-)

> diff --git a/testcases/kernel/syscalls/getxattr/getxattr02.c b/testcases/kernel/syscalls/getxattr/getxattr02.c
> index a42057d0a..dbbce8f16 100644
> --- a/testcases/kernel/syscalls/getxattr/getxattr02.c
> +++ b/testcases/kernel/syscalls/getxattr/getxattr02.c
> @@ -1,64 +1,42 @@
> +// 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 shows it should be GPL-2.0-only.

> - *
> - * 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) 2011  Red Hat, Inc.
nit: Test has been updated by LTP in 2012-2022.
> + * Copyright (c) 2023 Marius Kittler <mkittler@suse.de>
>   */

> -/*
> +/*\
> + * [Description]
> + *
>   * In the user.* namespace, only regular files and directories can
>   * have extended attributes. Otherwise getxattr(2) will return -1
>   * and set errno to ENODATA.
>   *
>   * There are 4 test cases:
There needs to be a blank line, otherwise test list will not be formated (it
will be inline)

> - * 1. Get attribute from a FIFO, setxattr(2) should return -1 and
> + * - Get attribute from a FIFO, setxattr(2) should return -1 and
>   *    set errno to ENODATA
> - * 2. Get attribute from a char special file, setxattr(2) should
> + * - Get attribute from a char special file, setxattr(2) should
>   *    return -1 and set errno to ENODATA
> - * 3. Get attribute from a block special file, setxattr(2) should
> + * - Get attribute from a block special file, setxattr(2) should
>   *    return -1 and set errno to ENODATA
> - * 4. Get attribute from a UNIX domain socket, setxattr(2) should
> + * - Get attribute from a UNIX domain socket, setxattr(2) should
>   *    return -1 and set errno to ENODATA
>   */

...
> +static struct test_case {
> +	char *fname;
> +	int mode;
> +} tcases[] = {
> +	{ /* case 00, get attr from fifo */
Maybe, instead of this comments, could you add
char *desc to struct test_case and,, instead of this comment, specify below:

> +	 .fname = FNAME FIFO,
> +	 .mode = S_IFIFO,
     .desc = "get attr from fifo"
> +	},
> +	{ /* case 01, get attr from char special */
> +	 .fname = FNAME CHR,
> +	 .mode = S_IFCHR,
> +	},
> +	{ /* case 02, get attr from block special */
> +	 .fname = FNAME BLK,
> +	 .mode = S_IFBLK,
> +	},
> +	{ /* case 03, get attr from UNIX domain socket */
> +	 .fname = FNAME SOCK,
> +	 .mode = S_IFSOCK,
> +	},
>  };

...
> +	struct test_case *tc = &tcases[i];
> +	dev_t dev = tc->mode == S_IFCHR ? makedev(1, 3) : 0u;
> +
> +	if (mknod(tc->fname, tc->mode | 0777, dev) < 0)
> +		tst_brk(TBROK | TERRNO, "create %s (mode %i) failed", tc->fname, tc->mode);
> +
> +	TEST(getxattr(tc->fname, XATTR_TEST_KEY, buf, BUFSIZ));
> +	if (TST_RET == -1 && TST_ERR == ENODATA)
> +		tst_res(TPASS | TTERRNO, "expected return value");
> +	else
> +		tst_res(TFAIL | TTERRNO, "unexpected return value"
> +				" - expected errno %d - got", ENODATA);
>  }

>  static void setup(void)
>  {
...
> +	/* assert xattr support in the current filesystem */
> +	SAFE_TOUCH(FNAME, 0644, NULL);
> +	SAFE_SETXATTR(FNAME, "user.test", "test", 4, XATTR_CREATE);
>  }

> -static void cleanup(void)
> -{
> -	tst_rmdir();
> -}
> -#else /* HAVE_SYS_XATTR_H */
> -int main(int argc, char *argv[])
> -{
> -	tst_brkm(TCONF, NULL, "<sys/xattr.h> does not exist.");
> -}
> +static struct tst_test test = {
> +	.all_filesystems = 1,
> +	.needs_root = 1,
> +	.mntpoint = MNTPOINT,
> +	.mount_device = 1,
> +	.skip_filesystems = (const char *const []) {
> +			"exfat",
> +			"tmpfs",
tmpfs got xattrs support in kernel 6.6
https://kernelnewbies.org/LinuxChanges#Linux_6.6.TMPFS
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2daf18a7884dc03d5164ab9c7dc3f2ea70638469
And test works on 6.7.0-rc3-3.ge7296f9-default (openSUSE):

LTP_SINGLE_FS_TYPE=tmpfs ./getxattr02
...
tst_test.c:1650: TINFO: === Testing on tmpfs ===
tst_test.c:1105: TINFO: Skipping mkfs for TMPFS filesystem
tst_test.c:1086: TINFO: Limiting tmpfs size to 32MB
tst_test.c:1119: TINFO: Mounting ltp-tmpfs to /tmp/LTP_get5FL6hO/mntpoint fstyp=tmpfs flags=0
getxattr02.c:80: TPASS: expected return value: ENODATA (61)
getxattr02.c:80: TPASS: expected return value: ENODATA (61)
getxattr02.c:80: TPASS: expected return value: ENODATA (61)
getxattr02.c:80: TPASS: expected return value: ENODATA (61)

And it TCONF on older kernels
getxattr02.c:90: TCONF: no xattr support in fs, mounted without user_xattr option or invalid namespace/name format
=> we should skip tmpfs.

vfat, exfat also detect correctly xattr. I don't know if they can even get
support (there might be a technical limitation of the format of their metadata),
but I would still keep them in the list. But maybe others will see it
differently.


> +			"ramfs",
> +			"nfs",
> +			"vfat",
> +			NULL
> +	},
> +	.setup = setup,
> +	.test = run,
> +	.tcnt = ARRAY_SIZE(tcases)
> +};
> +
> +#else
> +TST_TEST_TCONF("System doesn't have <sys/xattr.h>");
>  #endif

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