[LTP] [PATCH] open14: Rewrite to new API

[LTP] [PATCH] open14: Rewrite to new API

[Martin Doucha]

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 testcases/kernel/syscalls/open/open14.c | 276 +++++++++++-------------
 1 file changed, 131 insertions(+), 145 deletions(-)

diff --git a/testcases/kernel/syscalls/open/open14.c b/testcases/kernel/syscalls/open/open14.c
index 3ecb7e4fb..41e274e69 100644
--- a/testcases/kernel/syscalls/open/open14.c
+++ b/testcases/kernel/syscalls/open/open14.c
@@ -1,63 +1,45 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2015-2016 Oracle and/or its affiliates. All Rights Reserved.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation; either version 2 of
- * the License, or (at your option) any later version.
- *
- * 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.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program. If not, see <http://www.gnu.org/licenses/>.
- *
  * Author: Alexey Kodanev <alexey.kodanev@oracle.com>
- *
+ * Copyright (c) 2025 SUSE LLC <mdoucha@suse.cz>
  */
 
-#define _GNU_SOURCE
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <unistd.h>
-#include <errno.h>
+/*\
+ * Check the functionality of O_TMPFILE flag for open() syscall.
+ */
 
-#include "test.h"
-#include "safe_macros.h"
+#include "tst_test.h"
+#include "tst_safe_macros.h"
 #include "lapi/fcntl.h"
 
-char *TCID = "open14";
-int TST_TOTAL = 3;
-static ssize_t size;
+#define FILE_COUNT 100
+#define MNTPOINT "mntpoint"
+
 static char buf[1024];
+static int fds[FILE_COUNT];
+static const ssize_t size = sizeof(buf);
 static const ssize_t blocks_num = 4;
-static struct stat st;
-
-static void cleanup(void)
-{
-	tst_rmdir();
-}
 
 static void setup(void)
 {
-	tst_tmpdir();
+	int i;
 
-	size = sizeof(buf);
+	for (i = 0; i < FILE_COUNT; i++)
+		fds[i] = -1;
 
 	memset(buf, 1, size);
+	SAFE_CHDIR(MNTPOINT);
+	fds[0] = open(".", O_TMPFILE | O_RDWR, 0600);
 
-	int fd = open(".", O_TMPFILE | O_RDWR, 0600);
-
-	if (fd == -1) {
+	if (fds[0] == -1) {
 		if (errno == EISDIR || errno == ENOTSUP)
-			tst_brkm(TCONF, cleanup, "O_TMPFILE not supported");
+			tst_brk(TCONF, "O_TMPFILE not supported");
 
-		tst_brkm(TBROK | TERRNO, cleanup, "open() failed");
+		tst_brk(TBROK | TERRNO, "open() failed");
 	}
 
-	SAFE_CLOSE(cleanup, fd);
+	SAFE_CLOSE(fds[0]);
 }
 
 static void write_file(int fd)
@@ -65,183 +47,187 @@ static void write_file(int fd)
 	int i;
 
 	for (i = 0; i < blocks_num; ++i)
-		SAFE_WRITE(cleanup, SAFE_WRITE_ALL, fd, buf, size);
+		SAFE_WRITE(1, fd, buf, size);
 }
 
 void test01(void)
 {
-	int fd;
-	char path[PATH_MAX], tmp[PATH_MAX];
-
-	tst_resm(TINFO, "creating a file with O_TMPFILE flag");
-	fd = SAFE_OPEN(cleanup, ".", O_TMPFILE | O_RDWR, 0600);
-
-	tst_resm(TINFO, "writing data to the file");
-	write_file(fd);
+	struct stat st;
+	char path[PATH_MAX];
 
-	SAFE_FSTAT(cleanup, fd, &st);
-	tst_resm(TINFO, "file size is '%li'", (long)st.st_size);
+	tst_res(TINFO, "Testing creation and linking of single temp file");
+	fds[0] = SAFE_OPEN(".", O_TMPFILE | O_RDWR, 0600);
+	write_file(fds[0]);
+	SAFE_FSTAT(fds[0], &st);
 
 	if (st.st_size != blocks_num * size) {
-		tst_resm(TFAIL, "not expected size: '%li' != '%zu'",
+		tst_res(TFAIL, "Unexpected test file size: %li != %zu",
 			 (long)st.st_size, blocks_num * size);
-		SAFE_CLOSE(cleanup, fd);
-		return;
+	} else {
+		tst_res(TPASS, "Test file size is %li", (long)st.st_size);
 	}
 
-	tst_resm(TINFO, "looking for the file in '.'");
-	if (!tst_dir_is_empty(cleanup, ".", 1))
-		tst_brkm(TFAIL, cleanup, "found a file, this is not expected");
-	tst_resm(TINFO, "file not found, OK");
+	if (!tst_dir_is_empty(".", 1))
+		tst_res(TFAIL, "Test directory is not empty");
+	else
+		tst_res(TPASS, "Test directory is empty");
 
-	snprintf(path, PATH_MAX,  "/proc/self/fd/%d", fd);
-	SAFE_READLINK(cleanup, path, tmp, PATH_MAX);
+	snprintf(path, PATH_MAX, "/proc/self/fd/%d", fds[0]);
+	tst_res(TINFO, "Linking unnamed test file to 'tmpfile'");
+	SAFE_LINKAT(AT_FDCWD, path, AT_FDCWD, "tmpfile", AT_SYMLINK_FOLLOW);
 
-	tst_resm(TINFO, "renaming '%s' -> 'tmpfile'", tmp);
-	SAFE_LINKAT(cleanup, AT_FDCWD, path, AT_FDCWD, "tmpfile",
-		    AT_SYMLINK_FOLLOW);
-
-	if (tst_dir_is_empty(cleanup, ".", 1))
-		tst_brkm(TFAIL, cleanup, "file not found");
+	if (tst_dir_is_empty(".", 1)) {
+		tst_res(TFAIL, "Test directory is still empty");
+		SAFE_CLOSE(fds[0]);
+		return;
+	}
 
-	SAFE_UNLINK(cleanup, "tmpfile");
-	SAFE_CLOSE(cleanup, fd);
+	if (access("tmpfile", F_OK)) {
+		tst_res(TFAIL | TERRNO, "Linked test file not found");
+		SAFE_CLOSE(fds[0]);
+		return;
+	}
 
-	tst_resm(TPASS, "single file tests passed");
+	tst_res(TPASS, "Test file was linked correctly");
+	SAFE_UNLINK("tmpfile");
+	SAFE_CLOSE(fds[0]);
 }
 
-static void read_file(int fd)
+static int read_file(int fd)
 {
 	int i;
 	char tmp[size];
 
-	SAFE_LSEEK(cleanup, fd, 0, SEEK_SET);
+	SAFE_LSEEK(fd, 0, SEEK_SET);
 
 	for (i = 0; i < blocks_num; ++i) {
-		SAFE_READ(cleanup, 0, fd, tmp, size);
-		if (memcmp(buf, tmp, size))
-			tst_brkm(TFAIL, cleanup, "got unexepected data");
+		SAFE_READ(0, fd, tmp, size);
+
+		if (memcmp(buf, tmp, size)) {
+			tst_res(TFAIL, "got unexepected data");
+			return 1;
+		}
 	}
+
+	return 0;
 }
 
 static void test02(void)
 {
-	const int files_num = 100;
-	int i, fd[files_num];
+	int i, fails = 0;
 	char path[PATH_MAX];
 
-	tst_resm(TINFO, "create files in multiple directories");
-	for (i = 0; i < files_num; ++i) {
+	tst_res(TINFO, "Testing temp files in multiple directories");
+	for (i = 0; i < FILE_COUNT; ++i) {
 		snprintf(path, PATH_MAX, "tst02_%d", i);
-		SAFE_MKDIR(cleanup, path, 0700);
-		SAFE_CHDIR(cleanup, path);
-
-		fd[i] = SAFE_OPEN(cleanup, ".", O_TMPFILE | O_RDWR, 0600);
+		SAFE_MKDIR(path, 0700);
+		SAFE_CHDIR(path);
+		fds[i] = SAFE_OPEN(".", O_TMPFILE | O_RDWR, 0600);
 	}
 
-	tst_resm(TINFO, "removing test directories");
-	for (i = files_num - 1; i >= 0; --i) {
-		SAFE_CHDIR(cleanup, "..");
+	tst_res(TINFO, "Removing test directories");
+	for (i = FILE_COUNT - 1; i >= 0; --i) {
+		SAFE_CHDIR("..");
 		snprintf(path, PATH_MAX, "tst02_%d", i);
-		SAFE_RMDIR(cleanup, path);
+		SAFE_RMDIR(path);
 	}
 
-	tst_resm(TINFO, "writing/reading temporary files");
-	for (i = 0; i < files_num; ++i) {
-		write_file(fd[i]);
-		read_file(fd[i]);
+	tst_res(TINFO, "Writing and reading temporary files");
+	for (i = 0; i < FILE_COUNT; ++i) {
+		write_file(fds[i]);
+		fails += read_file(fds[i]);
 	}
 
-	tst_resm(TINFO, "closing temporary files");
-	for (i = 0; i < files_num; ++i)
-		SAFE_CLOSE(cleanup, fd[i]);
+	tst_res(TINFO, "Closing temporary files");
+	for (i = 0; i < FILE_COUNT; ++i)
+		SAFE_CLOSE(fds[i]);
 
-	tst_resm(TPASS, "multiple files tests passed");
+	if (!fails)
+		tst_res(TPASS, "Multiple files test passed");
 }
 
 static void link_tmp_file(int fd)
 {
 	char path1[PATH_MAX], path2[PATH_MAX];
 
-	snprintf(path1, PATH_MAX,  "/proc/self/fd/%d", fd);
-	snprintf(path2, PATH_MAX,  "tmpfile_%d", fd);
-
-	SAFE_LINKAT(cleanup, AT_FDCWD, path1, AT_FDCWD, path2,
-		    AT_SYMLINK_FOLLOW);
+	snprintf(path1, PATH_MAX, "/proc/self/fd/%d", fd);
+	snprintf(path2, PATH_MAX, "tmpfile_%d", fd);
+	SAFE_LINKAT(AT_FDCWD, path1, AT_FDCWD, path2, AT_SYMLINK_FOLLOW);
 }
 
 static void test03(void)
 {
-	const int files_num = 100;
 	const mode_t test_perms[] = { 0, 07777, 001, 0755, 0644, 0440 };
 
-	int i, fd[files_num];
+	int i, fails = 0;
 	char path[PATH_MAX];
 	struct stat st;
 	mode_t mask = umask(0), perm;
 
 	umask(mask);
+	tst_res(TINFO, "Testing linked temp files access mode");
 
-	tst_resm(TINFO, "create multiple directories, link files into them");
-	tst_resm(TINFO, "and check file permissions");
-	for (i = 0; i < files_num; ++i) {
-
+	for (i = 0; i < FILE_COUNT; ++i) {
 		snprintf(path, PATH_MAX, "tst03_%d", i);
-		SAFE_MKDIR(cleanup, path, 0700);
-		SAFE_CHDIR(cleanup, path);
+		SAFE_MKDIR(path, 0700);
+		SAFE_CHDIR(path);
 
 		perm = test_perms[i % ARRAY_SIZE(test_perms)];
-
-		fd[i] = SAFE_OPEN(cleanup, ".", O_TMPFILE | O_RDWR, perm);
-
-		write_file(fd[i]);
-		read_file(fd[i]);
-
-		link_tmp_file(fd[i]);
-
-		snprintf(path, PATH_MAX, "tmpfile_%d", fd[i]);
-
-		SAFE_LSTAT(cleanup, path, &st);
-
-		mode_t exp_mode = perm & ~mask;
-
-		if ((st.st_mode & ~S_IFMT) != exp_mode) {
-			tst_brkm(TFAIL, cleanup,
-				"file mode read %o, but expected %o",
-				st.st_mode & ~S_IFMT, exp_mode);
+		fds[i] = SAFE_OPEN(".", O_TMPFILE | O_RDWR, perm);
+		write_file(fds[i]);
+		read_file(fds[i]);
+		link_tmp_file(fds[i]);
+
+		snprintf(path, PATH_MAX, "tmpfile_%d", fds[i]);
+		SAFE_LSTAT(path, &st);
+		perm &= ~mask;
+
+		if ((st.st_mode & ~S_IFMT) != perm) {
+			tst_res(TFAIL, "Unexpected access mode: %04o != %04o",
+				st.st_mode & ~S_IFMT, perm);
+			fails++;
 		}
 	}
 
-	tst_resm(TINFO, "remove files, directories");
-	for (i = files_num - 1; i >= 0; --i) {
-		snprintf(path, PATH_MAX, "tmpfile_%d", fd[i]);
-		SAFE_UNLINK(cleanup, path);
-		SAFE_CLOSE(cleanup, fd[i]);
+	if (!fails)
+		tst_res(TPASS, "File access modes are correct");
+
+	tst_res(TINFO, "Removing files and directories");
 
-		SAFE_CHDIR(cleanup, "..");
+	for (i = FILE_COUNT - 1; i >= 0; --i) {
+		snprintf(path, PATH_MAX, "tmpfile_%d", fds[i]);
+		SAFE_UNLINK(path);
+		SAFE_CLOSE(fds[i]);
 
+		SAFE_CHDIR("..");
 		snprintf(path, PATH_MAX, "tst03_%d", i);
-		SAFE_RMDIR(cleanup, path);
+		SAFE_RMDIR(path);
 	}
-
-	tst_resm(TPASS, "file permission tests passed");
 }
 
-int main(int ac, char *av[])
+static void run(void)
 {
-	int lc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
+	test01();
+	test02();
+	test03();
+}
 
-	setup();
+static void cleanup(void)
+{
+	int i;
 
-	for (lc = 0; TEST_LOOPING(lc); ++lc) {
-		tst_count = 0;
-		test01();
-		test02();
-		test03();
+	for (i = 0; i < FILE_COUNT; i++) {
+		if (fds[i] >= 0)
+			SAFE_CLOSE(fds[i]);
 	}
 
-	cleanup();
-	tst_exit();
+	SAFE_CHDIR("..");
 }
+
+static struct tst_test test = {
+	.setup = setup,
+	.test_all = run,
+	.cleanup = cleanup,
+	.mntpoint = MNTPOINT,
+	.all_filesystems = 1
+};
-- 
2.50.1


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

Re: [LTP] [PATCH] open14: Rewrite to new API

[Andrea Cervesato via ltp]

Hi!

On 7/18/25 6:29 PM, Martin Doucha wrote:
> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
> ---
>   testcases/kernel/syscalls/open/open14.c | 276 +++++++++++-------------
>   1 file changed, 131 insertions(+), 145 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/open/open14.c b/testcases/kernel/syscalls/open/open14.c
> index 3ecb7e4fb..41e274e69 100644
> --- a/testcases/kernel/syscalls/open/open14.c
> +++ b/testcases/kernel/syscalls/open/open14.c
> @@ -1,63 +1,45 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
>   /*
>    * Copyright (c) 2015-2016 Oracle and/or its affiliates. All Rights Reserved.
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public License as
> - * published by the Free Software Foundation; either version 2 of
> - * the License, or (at your option) any later version.
> - *
> - * 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.  See the
> - * GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program. If not, see <http://www.gnu.org/licenses/>.
> - *
>    * Author: Alexey Kodanev <alexey.kodanev@oracle.com>
> - *
> + * Copyright (c) 2025 SUSE LLC <mdoucha@suse.cz>
>    */
>   
> -#define _GNU_SOURCE
> -#include <sys/types.h>
> -#include <sys/stat.h>
> -#include <unistd.h>
> -#include <errno.h>
> +/*\
> + * Check the functionality of O_TMPFILE flag for open() syscall.
It would be nice to add the 3 test cases in the description.
> + */
>   
> -#include "test.h"
> -#include "safe_macros.h"
> +#include "tst_test.h"
> +#include "tst_safe_macros.h"
>   #include "lapi/fcntl.h"
>   
> -char *TCID = "open14";
> -int TST_TOTAL = 3;
> -static ssize_t size;
> +#define FILE_COUNT 100
> +#define MNTPOINT "mntpoint"
> +
>   static char buf[1024];
> +static int fds[FILE_COUNT];
Since we are not using variants, we can initialize all items ad 
declaration without doing it inside the setup:

static char buf[1024] ={1};
static int fds[FILE_COUNT] = {-1};

But probably buf can be initialized using tst_rand_data.
> +static const ssize_t size = sizeof(buf);
Why not defining a macro with the size of the buf and use that one 
inside the code?

#define BUFSIZE 1024
>   static const ssize_t blocks_num = 4;
> -static struct stat st;
> -
> -static void cleanup(void)
> -{
> -	tst_rmdir();
> -}
>   
>   static void setup(void)
>   {
> -	tst_tmpdir();
> +	int i;
In c99 we can define int inside the for loop.
>   
> -	size = sizeof(buf);
> +	for (i = 0; i < FILE_COUNT; i++)
> +		fds[i] = -1;
>   
>   	memset(buf, 1, size);
> +	SAFE_CHDIR(MNTPOINT);
> +	fds[0] = open(".", O_TMPFILE | O_RDWR, 0600);
>   
> -	int fd = open(".", O_TMPFILE | O_RDWR, 0600);
> -
> -	if (fd == -1) {
> +	if (fds[0] == -1) {
>   		if (errno == EISDIR || errno == ENOTSUP)
> -			tst_brkm(TCONF, cleanup, "O_TMPFILE not supported");
> +			tst_brk(TCONF, "O_TMPFILE not supported");
>   
> -		tst_brkm(TBROK | TERRNO, cleanup, "open() failed");
> +		tst_brk(TBROK | TERRNO, "open() failed");
>   	}
>   
> -	SAFE_CLOSE(cleanup, fd);
> +	SAFE_CLOSE(fds[0]);
>   }
>   
>   static void write_file(int fd)
> @@ -65,183 +47,187 @@ static void write_file(int fd)
>   	int i;
>   
>   	for (i = 0; i < blocks_num; ++i)
> -		SAFE_WRITE(cleanup, SAFE_WRITE_ALL, fd, buf, size);
> +		SAFE_WRITE(1, fd, buf, size);
>   }
>   
>   void test01(void)
>   {
> -	int fd;
> -	char path[PATH_MAX], tmp[PATH_MAX];
> -
> -	tst_resm(TINFO, "creating a file with O_TMPFILE flag");
> -	fd = SAFE_OPEN(cleanup, ".", O_TMPFILE | O_RDWR, 0600);
> -
> -	tst_resm(TINFO, "writing data to the file");
> -	write_file(fd);
> +	struct stat st;
> +	char path[PATH_MAX];
>   
> -	SAFE_FSTAT(cleanup, fd, &st);
> -	tst_resm(TINFO, "file size is '%li'", (long)st.st_size);
> +	tst_res(TINFO, "Testing creation and linking of single temp file");
> +	fds[0] = SAFE_OPEN(".", O_TMPFILE | O_RDWR, 0600);
> +	write_file(fds[0]);
> +	SAFE_FSTAT(fds[0], &st);
>   
>   	if (st.st_size != blocks_num * size) {
> -		tst_resm(TFAIL, "not expected size: '%li' != '%zu'",
> +		tst_res(TFAIL, "Unexpected test file size: %li != %zu",
>   			 (long)st.st_size, blocks_num * size);
> -		SAFE_CLOSE(cleanup, fd);
> -		return;
> +	} else {
> +		tst_res(TPASS, "Test file size is %li", (long)st.st_size);
>   	}
>   
> -	tst_resm(TINFO, "looking for the file in '.'");
> -	if (!tst_dir_is_empty(cleanup, ".", 1))
> -		tst_brkm(TFAIL, cleanup, "found a file, this is not expected");
> -	tst_resm(TINFO, "file not found, OK");
> +	if (!tst_dir_is_empty(".", 1))
> +		tst_res(TFAIL, "Test directory is not empty");
> +	else
> +		tst_res(TPASS, "Test directory is empty");
>   
> -	snprintf(path, PATH_MAX,  "/proc/self/fd/%d", fd);
> -	SAFE_READLINK(cleanup, path, tmp, PATH_MAX);
> +	snprintf(path, PATH_MAX, "/proc/self/fd/%d", fds[0]);
> +	tst_res(TINFO, "Linking unnamed test file to 'tmpfile'");
> +	SAFE_LINKAT(AT_FDCWD, path, AT_FDCWD, "tmpfile", AT_SYMLINK_FOLLOW);
>   
> -	tst_resm(TINFO, "renaming '%s' -> 'tmpfile'", tmp);
> -	SAFE_LINKAT(cleanup, AT_FDCWD, path, AT_FDCWD, "tmpfile",
> -		    AT_SYMLINK_FOLLOW);
> -
> -	if (tst_dir_is_empty(cleanup, ".", 1))
> -		tst_brkm(TFAIL, cleanup, "file not found");
> +	if (tst_dir_is_empty(".", 1)) {
> +		tst_res(TFAIL, "Test directory is still empty");
> +		SAFE_CLOSE(fds[0]);
> +		return;
> +	}
>   
> -	SAFE_UNLINK(cleanup, "tmpfile");
> -	SAFE_CLOSE(cleanup, fd);
> +	if (access("tmpfile", F_OK)) {
> +		tst_res(TFAIL | TERRNO, "Linked test file not found");
> +		SAFE_CLOSE(fds[0]);
> +		return;
> +	}
>   
> -	tst_resm(TPASS, "single file tests passed");
> +	tst_res(TPASS, "Test file was linked correctly");
> +	SAFE_UNLINK("tmpfile");
> +	SAFE_CLOSE(fds[0]);
>   }
>   
> -static void read_file(int fd)
> +static int read_file(int fd)
>   {
>   	int i;
>   	char tmp[size];
>   
> -	SAFE_LSEEK(cleanup, fd, 0, SEEK_SET);
> +	SAFE_LSEEK(fd, 0, SEEK_SET);
>   
>   	for (i = 0; i < blocks_num; ++i) {
> -		SAFE_READ(cleanup, 0, fd, tmp, size);
> -		if (memcmp(buf, tmp, size))
> -			tst_brkm(TFAIL, cleanup, "got unexepected data");
> +		SAFE_READ(0, fd, tmp, size);
> +
> +		if (memcmp(buf, tmp, size)) {
> +			tst_res(TFAIL, "got unexepected data");
> +			return 1;
> +		}
>   	}
> +
> +	return 0;
>   }
>   
>   static void test02(void)
>   {
> -	const int files_num = 100;
> -	int i, fd[files_num];
> +	int i, fails = 0;
>   	char path[PATH_MAX];
>   
> -	tst_resm(TINFO, "create files in multiple directories");
> -	for (i = 0; i < files_num; ++i) {
> +	tst_res(TINFO, "Testing temp files in multiple directories");
> +	for (i = 0; i < FILE_COUNT; ++i) {
>   		snprintf(path, PATH_MAX, "tst02_%d", i);
> -		SAFE_MKDIR(cleanup, path, 0700);
> -		SAFE_CHDIR(cleanup, path);
> -
> -		fd[i] = SAFE_OPEN(cleanup, ".", O_TMPFILE | O_RDWR, 0600);
> +		SAFE_MKDIR(path, 0700);
> +		SAFE_CHDIR(path);
> +		fds[i] = SAFE_OPEN(".", O_TMPFILE | O_RDWR, 0600);
>   	}
>   
> -	tst_resm(TINFO, "removing test directories");
> -	for (i = files_num - 1; i >= 0; --i) {
> -		SAFE_CHDIR(cleanup, "..");
> +	tst_res(TINFO, "Removing test directories");
> +	for (i = FILE_COUNT - 1; i >= 0; --i) {
> +		SAFE_CHDIR("..");
>   		snprintf(path, PATH_MAX, "tst02_%d", i);
> -		SAFE_RMDIR(cleanup, path);
> +		SAFE_RMDIR(path);
>   	}
>   
> -	tst_resm(TINFO, "writing/reading temporary files");
> -	for (i = 0; i < files_num; ++i) {
> -		write_file(fd[i]);
> -		read_file(fd[i]);
> +	tst_res(TINFO, "Writing and reading temporary files");
> +	for (i = 0; i < FILE_COUNT; ++i) {
> +		write_file(fds[i]);
> +		fails += read_file(fds[i]);
>   	}
>   
> -	tst_resm(TINFO, "closing temporary files");
> -	for (i = 0; i < files_num; ++i)
> -		SAFE_CLOSE(cleanup, fd[i]);
> +	tst_res(TINFO, "Closing temporary files");
> +	for (i = 0; i < FILE_COUNT; ++i)
> +		SAFE_CLOSE(fds[i]);
>   
> -	tst_resm(TPASS, "multiple files tests passed");
> +	if (!fails)
> +		tst_res(TPASS, "Multiple files test passed");
>   }
>   
>   static void link_tmp_file(int fd)
>   {
>   	char path1[PATH_MAX], path2[PATH_MAX];
>   
> -	snprintf(path1, PATH_MAX,  "/proc/self/fd/%d", fd);
> -	snprintf(path2, PATH_MAX,  "tmpfile_%d", fd);
> -
> -	SAFE_LINKAT(cleanup, AT_FDCWD, path1, AT_FDCWD, path2,
> -		    AT_SYMLINK_FOLLOW);
> +	snprintf(path1, PATH_MAX, "/proc/self/fd/%d", fd);
> +	snprintf(path2, PATH_MAX, "tmpfile_%d", fd);
> +	SAFE_LINKAT(AT_FDCWD, path1, AT_FDCWD, path2, AT_SYMLINK_FOLLOW);
>   }
>   
>   static void test03(void)
>   {
> -	const int files_num = 100;
>   	const mode_t test_perms[] = { 0, 07777, 001, 0755, 0644, 0440 };
>   
> -	int i, fd[files_num];
> +	int i, fails = 0;
>   	char path[PATH_MAX];
>   	struct stat st;
>   	mode_t mask = umask(0), perm;
>   
>   	umask(mask);
> +	tst_res(TINFO, "Testing linked temp files access mode");
>   
> -	tst_resm(TINFO, "create multiple directories, link files into them");
> -	tst_resm(TINFO, "and check file permissions");
> -	for (i = 0; i < files_num; ++i) {
> -
> +	for (i = 0; i < FILE_COUNT; ++i) {
>   		snprintf(path, PATH_MAX, "tst03_%d", i);
> -		SAFE_MKDIR(cleanup, path, 0700);
> -		SAFE_CHDIR(cleanup, path);
> +		SAFE_MKDIR(path, 0700);
> +		SAFE_CHDIR(path);
>   
>   		perm = test_perms[i % ARRAY_SIZE(test_perms)];
> -
> -		fd[i] = SAFE_OPEN(cleanup, ".", O_TMPFILE | O_RDWR, perm);
> -
> -		write_file(fd[i]);
> -		read_file(fd[i]);
> -
> -		link_tmp_file(fd[i]);
> -
> -		snprintf(path, PATH_MAX, "tmpfile_%d", fd[i]);
> -
> -		SAFE_LSTAT(cleanup, path, &st);
> -
> -		mode_t exp_mode = perm & ~mask;
> -
> -		if ((st.st_mode & ~S_IFMT) != exp_mode) {
> -			tst_brkm(TFAIL, cleanup,
> -				"file mode read %o, but expected %o",
> -				st.st_mode & ~S_IFMT, exp_mode);
> +		fds[i] = SAFE_OPEN(".", O_TMPFILE | O_RDWR, perm);
> +		write_file(fds[i]);
> +		read_file(fds[i]);
> +		link_tmp_file(fds[i]);
> +
> +		snprintf(path, PATH_MAX, "tmpfile_%d", fds[i]);
> +		SAFE_LSTAT(path, &st);
> +		perm &= ~mask;
> +
> +		if ((st.st_mode & ~S_IFMT) != perm) {
> +			tst_res(TFAIL, "Unexpected access mode: %04o != %04o",
> +				st.st_mode & ~S_IFMT, perm);
> +			fails++;
>   		}
>   	}
>   
> -	tst_resm(TINFO, "remove files, directories");
> -	for (i = files_num - 1; i >= 0; --i) {
> -		snprintf(path, PATH_MAX, "tmpfile_%d", fd[i]);
> -		SAFE_UNLINK(cleanup, path);
> -		SAFE_CLOSE(cleanup, fd[i]);
> +	if (!fails)
> +		tst_res(TPASS, "File access modes are correct");
> +
> +	tst_res(TINFO, "Removing files and directories");
>   
> -		SAFE_CHDIR(cleanup, "..");
> +	for (i = FILE_COUNT - 1; i >= 0; --i) {
> +		snprintf(path, PATH_MAX, "tmpfile_%d", fds[i]);
> +		SAFE_UNLINK(path);
> +		SAFE_CLOSE(fds[i]);
>   
> +		SAFE_CHDIR("..");
>   		snprintf(path, PATH_MAX, "tst03_%d", i);
> -		SAFE_RMDIR(cleanup, path);
> +		SAFE_RMDIR(path);
>   	}
> -
> -	tst_resm(TPASS, "file permission tests passed");
>   }
>   
> -int main(int ac, char *av[])
> +static void run(void)
>   {
> -	int lc;
> -
> -	tst_parse_opts(ac, av, NULL, NULL);
> +	test01();
> +	test02();
> +	test03();
> +}
>   
> -	setup();
> +static void cleanup(void)
> +{
> +	int i;
>   
> -	for (lc = 0; TEST_LOOPING(lc); ++lc) {
> -		tst_count = 0;
> -		test01();
> -		test02();
> -		test03();
> +	for (i = 0; i < FILE_COUNT; i++) {
> +		if (fds[i] >= 0)
> +			SAFE_CLOSE(fds[i]);
>   	}
>   
> -	cleanup();
> -	tst_exit();
> +	SAFE_CHDIR("..");
>   }
> +
> +static struct tst_test test = {
> +	.setup = setup,
> +	.test_all = run,
> +	.cleanup = cleanup,
> +	.mntpoint = MNTPOINT,
> +	.all_filesystems = 1
We need .needs_root if we acquire a device.
> +};

The rest looks ok, but I'm wondering if we really need to call 
SAFE_CHDIR() so many times before SAFE_OPEN(".", ...) instead of passing 
the full path. We always obtain a file descriptor and in general we 
would like to reduce calls which are not 100% needed by the tests.

- Andrea

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

Re: [LTP] [PATCH] open14: Rewrite to new API

[Martin Doucha]

On 21. 07. 25 15:56, Andrea Cervesato wrote:
> Hi!
> 
> On 7/18/25 6:29 PM, Martin Doucha wrote:
>> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
>> ---
>>   testcases/kernel/syscalls/open/open14.c | 276 +++++++++++-------------
>>   1 file changed, 131 insertions(+), 145 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/open/open14.c b/testcases/ 
>> kernel/syscalls/open/open14.c
>> index 3ecb7e4fb..41e274e69 100644
>> --- a/testcases/kernel/syscalls/open/open14.c
>> +++ b/testcases/kernel/syscalls/open/open14.c
>> @@ -1,63 +1,45 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>   /*
>>    * Copyright (c) 2015-2016 Oracle and/or its affiliates. All Rights 
>> Reserved.
>> - *
>> - * This program is free software; you can redistribute it and/or
>> - * modify it under the terms of the GNU General Public License as
>> - * published by the Free Software Foundation; either version 2 of
>> - * the License, or (at your option) any later version.
>> - *
>> - * 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.  See the
>> - * GNU General Public License for more details.
>> - *
>> - * You should have received a copy of the GNU General Public License
>> - * along with this program. If not, see <http://www.gnu.org/licenses/>.
>> - *
>>    * Author: Alexey Kodanev <alexey.kodanev@oracle.com>
>> - *
>> + * Copyright (c) 2025 SUSE LLC <mdoucha@suse.cz>
>>    */
>> -#define _GNU_SOURCE
>> -#include <sys/types.h>
>> -#include <sys/stat.h>
>> -#include <unistd.h>
>> -#include <errno.h>
>> +/*\
>> + * Check the functionality of O_TMPFILE flag for open() syscall.
> It would be nice to add the 3 test cases in the description.

OK, I'll send v2 soon.

>> + */
>> -#include "test.h"
>> -#include "safe_macros.h"
>> +#include "tst_test.h"
>> +#include "tst_safe_macros.h"
>>   #include "lapi/fcntl.h"
>> -char *TCID = "open14";
>> -int TST_TOTAL = 3;
>> -static ssize_t size;
>> +#define FILE_COUNT 100
>> +#define MNTPOINT "mntpoint"
>> +
>>   static char buf[1024];
>> +static int fds[FILE_COUNT];
> Since we are not using variants, we can initialize all items ad 
> declaration without doing it inside the setup:
> 
> static char buf[1024] ={1};
> static int fds[FILE_COUNT] = {-1};
> 
> But probably buf can be initialized using tst_rand_data.

This would only initialize the first element and fill the rest of the 
array with zeroes. Initializing the arrays in setup() is the correct 
approach.

>> +static const ssize_t size = sizeof(buf);
> Why not defining a macro with the size of the buf and use that one 
> inside the code?
> 
> #define BUFSIZE 1024

There's no practical difference so I prefer to leave the original code 
as is.

>>   static const ssize_t blocks_num = 4;
>> -static struct stat st;
>> -
>> -static void cleanup(void)
>> -{
>> -    tst_rmdir();
>> -}
>>   static void setup(void)
>>   {
>> -    tst_tmpdir();
>> +    int i;
> In c99 we can define int inside the for loop.

We can, but I chose not to.

>> -    size = sizeof(buf);
>> +    for (i = 0; i < FILE_COUNT; i++)
>> +        fds[i] = -1;
>>       memset(buf, 1, size);
>> +    SAFE_CHDIR(MNTPOINT);
>> +    fds[0] = open(".", O_TMPFILE | O_RDWR, 0600);
>> -    int fd = open(".", O_TMPFILE | O_RDWR, 0600);
>> -
>> -    if (fd == -1) {
>> +    if (fds[0] == -1) {
>>           if (errno == EISDIR || errno == ENOTSUP)
>> -            tst_brkm(TCONF, cleanup, "O_TMPFILE not supported");
>> +            tst_brk(TCONF, "O_TMPFILE not supported");
>> -        tst_brkm(TBROK | TERRNO, cleanup, "open() failed");
>> +        tst_brk(TBROK | TERRNO, "open() failed");
>>       }
>> -    SAFE_CLOSE(cleanup, fd);
>> +    SAFE_CLOSE(fds[0]);
>>   }
>>   static void write_file(int fd)
>> @@ -65,183 +47,187 @@ static void write_file(int fd)
>>       int i;
>>       for (i = 0; i < blocks_num; ++i)
>> -        SAFE_WRITE(cleanup, SAFE_WRITE_ALL, fd, buf, size);
>> +        SAFE_WRITE(1, fd, buf, size);
>>   }
>>   void test01(void)
>>   {
>> -    int fd;
>> -    char path[PATH_MAX], tmp[PATH_MAX];
>> -
>> -    tst_resm(TINFO, "creating a file with O_TMPFILE flag");
>> -    fd = SAFE_OPEN(cleanup, ".", O_TMPFILE | O_RDWR, 0600);
>> -
>> -    tst_resm(TINFO, "writing data to the file");
>> -    write_file(fd);
>> +    struct stat st;
>> +    char path[PATH_MAX];
>> -    SAFE_FSTAT(cleanup, fd, &st);
>> -    tst_resm(TINFO, "file size is '%li'", (long)st.st_size);
>> +    tst_res(TINFO, "Testing creation and linking of single temp file");
>> +    fds[0] = SAFE_OPEN(".", O_TMPFILE | O_RDWR, 0600);
>> +    write_file(fds[0]);
>> +    SAFE_FSTAT(fds[0], &st);
>>       if (st.st_size != blocks_num * size) {
>> -        tst_resm(TFAIL, "not expected size: '%li' != '%zu'",
>> +        tst_res(TFAIL, "Unexpected test file size: %li != %zu",
>>                (long)st.st_size, blocks_num * size);
>> -        SAFE_CLOSE(cleanup, fd);
>> -        return;
>> +    } else {
>> +        tst_res(TPASS, "Test file size is %li", (long)st.st_size);
>>       }
>> -    tst_resm(TINFO, "looking for the file in '.'");
>> -    if (!tst_dir_is_empty(cleanup, ".", 1))
>> -        tst_brkm(TFAIL, cleanup, "found a file, this is not expected");
>> -    tst_resm(TINFO, "file not found, OK");
>> +    if (!tst_dir_is_empty(".", 1))
>> +        tst_res(TFAIL, "Test directory is not empty");
>> +    else
>> +        tst_res(TPASS, "Test directory is empty");
>> -    snprintf(path, PATH_MAX,  "/proc/self/fd/%d", fd);
>> -    SAFE_READLINK(cleanup, path, tmp, PATH_MAX);
>> +    snprintf(path, PATH_MAX, "/proc/self/fd/%d", fds[0]);
>> +    tst_res(TINFO, "Linking unnamed test file to 'tmpfile'");
>> +    SAFE_LINKAT(AT_FDCWD, path, AT_FDCWD, "tmpfile", AT_SYMLINK_FOLLOW);
>> -    tst_resm(TINFO, "renaming '%s' -> 'tmpfile'", tmp);
>> -    SAFE_LINKAT(cleanup, AT_FDCWD, path, AT_FDCWD, "tmpfile",
>> -            AT_SYMLINK_FOLLOW);
>> -
>> -    if (tst_dir_is_empty(cleanup, ".", 1))
>> -        tst_brkm(TFAIL, cleanup, "file not found");
>> +    if (tst_dir_is_empty(".", 1)) {
>> +        tst_res(TFAIL, "Test directory is still empty");
>> +        SAFE_CLOSE(fds[0]);
>> +        return;
>> +    }
>> -    SAFE_UNLINK(cleanup, "tmpfile");
>> -    SAFE_CLOSE(cleanup, fd);
>> +    if (access("tmpfile", F_OK)) {
>> +        tst_res(TFAIL | TERRNO, "Linked test file not found");
>> +        SAFE_CLOSE(fds[0]);
>> +        return;
>> +    }
>> -    tst_resm(TPASS, "single file tests passed");
>> +    tst_res(TPASS, "Test file was linked correctly");
>> +    SAFE_UNLINK("tmpfile");
>> +    SAFE_CLOSE(fds[0]);
>>   }
>> -static void read_file(int fd)
>> +static int read_file(int fd)
>>   {
>>       int i;
>>       char tmp[size];
>> -    SAFE_LSEEK(cleanup, fd, 0, SEEK_SET);
>> +    SAFE_LSEEK(fd, 0, SEEK_SET);
>>       for (i = 0; i < blocks_num; ++i) {
>> -        SAFE_READ(cleanup, 0, fd, tmp, size);
>> -        if (memcmp(buf, tmp, size))
>> -            tst_brkm(TFAIL, cleanup, "got unexepected data");
>> +        SAFE_READ(0, fd, tmp, size);
>> +
>> +        if (memcmp(buf, tmp, size)) {
>> +            tst_res(TFAIL, "got unexepected data");
>> +            return 1;
>> +        }
>>       }
>> +
>> +    return 0;
>>   }
>>   static void test02(void)
>>   {
>> -    const int files_num = 100;
>> -    int i, fd[files_num];
>> +    int i, fails = 0;
>>       char path[PATH_MAX];
>> -    tst_resm(TINFO, "create files in multiple directories");
>> -    for (i = 0; i < files_num; ++i) {
>> +    tst_res(TINFO, "Testing temp files in multiple directories");
>> +    for (i = 0; i < FILE_COUNT; ++i) {
>>           snprintf(path, PATH_MAX, "tst02_%d", i);
>> -        SAFE_MKDIR(cleanup, path, 0700);
>> -        SAFE_CHDIR(cleanup, path);
>> -
>> -        fd[i] = SAFE_OPEN(cleanup, ".", O_TMPFILE | O_RDWR, 0600);
>> +        SAFE_MKDIR(path, 0700);
>> +        SAFE_CHDIR(path);
>> +        fds[i] = SAFE_OPEN(".", O_TMPFILE | O_RDWR, 0600);
>>       }
>> -    tst_resm(TINFO, "removing test directories");
>> -    for (i = files_num - 1; i >= 0; --i) {
>> -        SAFE_CHDIR(cleanup, "..");
>> +    tst_res(TINFO, "Removing test directories");
>> +    for (i = FILE_COUNT - 1; i >= 0; --i) {
>> +        SAFE_CHDIR("..");
>>           snprintf(path, PATH_MAX, "tst02_%d", i);
>> -        SAFE_RMDIR(cleanup, path);
>> +        SAFE_RMDIR(path);
>>       }
>> -    tst_resm(TINFO, "writing/reading temporary files");
>> -    for (i = 0; i < files_num; ++i) {
>> -        write_file(fd[i]);
>> -        read_file(fd[i]);
>> +    tst_res(TINFO, "Writing and reading temporary files");
>> +    for (i = 0; i < FILE_COUNT; ++i) {
>> +        write_file(fds[i]);
>> +        fails += read_file(fds[i]);
>>       }
>> -    tst_resm(TINFO, "closing temporary files");
>> -    for (i = 0; i < files_num; ++i)
>> -        SAFE_CLOSE(cleanup, fd[i]);
>> +    tst_res(TINFO, "Closing temporary files");
>> +    for (i = 0; i < FILE_COUNT; ++i)
>> +        SAFE_CLOSE(fds[i]);
>> -    tst_resm(TPASS, "multiple files tests passed");
>> +    if (!fails)
>> +        tst_res(TPASS, "Multiple files test passed");
>>   }
>>   static void link_tmp_file(int fd)
>>   {
>>       char path1[PATH_MAX], path2[PATH_MAX];
>> -    snprintf(path1, PATH_MAX,  "/proc/self/fd/%d", fd);
>> -    snprintf(path2, PATH_MAX,  "tmpfile_%d", fd);
>> -
>> -    SAFE_LINKAT(cleanup, AT_FDCWD, path1, AT_FDCWD, path2,
>> -            AT_SYMLINK_FOLLOW);
>> +    snprintf(path1, PATH_MAX, "/proc/self/fd/%d", fd);
>> +    snprintf(path2, PATH_MAX, "tmpfile_%d", fd);
>> +    SAFE_LINKAT(AT_FDCWD, path1, AT_FDCWD, path2, AT_SYMLINK_FOLLOW);
>>   }
>>   static void test03(void)
>>   {
>> -    const int files_num = 100;
>>       const mode_t test_perms[] = { 0, 07777, 001, 0755, 0644, 0440 };
>> -    int i, fd[files_num];
>> +    int i, fails = 0;
>>       char path[PATH_MAX];
>>       struct stat st;
>>       mode_t mask = umask(0), perm;
>>       umask(mask);
>> +    tst_res(TINFO, "Testing linked temp files access mode");
>> -    tst_resm(TINFO, "create multiple directories, link files into 
>> them");
>> -    tst_resm(TINFO, "and check file permissions");
>> -    for (i = 0; i < files_num; ++i) {
>> -
>> +    for (i = 0; i < FILE_COUNT; ++i) {
>>           snprintf(path, PATH_MAX, "tst03_%d", i);
>> -        SAFE_MKDIR(cleanup, path, 0700);
>> -        SAFE_CHDIR(cleanup, path);
>> +        SAFE_MKDIR(path, 0700);
>> +        SAFE_CHDIR(path);
>>           perm = test_perms[i % ARRAY_SIZE(test_perms)];
>> -
>> -        fd[i] = SAFE_OPEN(cleanup, ".", O_TMPFILE | O_RDWR, perm);
>> -
>> -        write_file(fd[i]);
>> -        read_file(fd[i]);
>> -
>> -        link_tmp_file(fd[i]);
>> -
>> -        snprintf(path, PATH_MAX, "tmpfile_%d", fd[i]);
>> -
>> -        SAFE_LSTAT(cleanup, path, &st);
>> -
>> -        mode_t exp_mode = perm & ~mask;
>> -
>> -        if ((st.st_mode & ~S_IFMT) != exp_mode) {
>> -            tst_brkm(TFAIL, cleanup,
>> -                "file mode read %o, but expected %o",
>> -                st.st_mode & ~S_IFMT, exp_mode);
>> +        fds[i] = SAFE_OPEN(".", O_TMPFILE | O_RDWR, perm);
>> +        write_file(fds[i]);
>> +        read_file(fds[i]);
>> +        link_tmp_file(fds[i]);
>> +
>> +        snprintf(path, PATH_MAX, "tmpfile_%d", fds[i]);
>> +        SAFE_LSTAT(path, &st);
>> +        perm &= ~mask;
>> +
>> +        if ((st.st_mode & ~S_IFMT) != perm) {
>> +            tst_res(TFAIL, "Unexpected access mode: %04o != %04o",
>> +                st.st_mode & ~S_IFMT, perm);
>> +            fails++;
>>           }
>>       }
>> -    tst_resm(TINFO, "remove files, directories");
>> -    for (i = files_num - 1; i >= 0; --i) {
>> -        snprintf(path, PATH_MAX, "tmpfile_%d", fd[i]);
>> -        SAFE_UNLINK(cleanup, path);
>> -        SAFE_CLOSE(cleanup, fd[i]);
>> +    if (!fails)
>> +        tst_res(TPASS, "File access modes are correct");
>> +
>> +    tst_res(TINFO, "Removing files and directories");
>> -        SAFE_CHDIR(cleanup, "..");
>> +    for (i = FILE_COUNT - 1; i >= 0; --i) {
>> +        snprintf(path, PATH_MAX, "tmpfile_%d", fds[i]);
>> +        SAFE_UNLINK(path);
>> +        SAFE_CLOSE(fds[i]);
>> +        SAFE_CHDIR("..");
>>           snprintf(path, PATH_MAX, "tst03_%d", i);
>> -        SAFE_RMDIR(cleanup, path);
>> +        SAFE_RMDIR(path);
>>       }
>> -
>> -    tst_resm(TPASS, "file permission tests passed");
>>   }
>> -int main(int ac, char *av[])
>> +static void run(void)
>>   {
>> -    int lc;
>> -
>> -    tst_parse_opts(ac, av, NULL, NULL);
>> +    test01();
>> +    test02();
>> +    test03();
>> +}
>> -    setup();
>> +static void cleanup(void)
>> +{
>> +    int i;
>> -    for (lc = 0; TEST_LOOPING(lc); ++lc) {
>> -        tst_count = 0;
>> -        test01();
>> -        test02();
>> -        test03();
>> +    for (i = 0; i < FILE_COUNT; i++) {
>> +        if (fds[i] >= 0)
>> +            SAFE_CLOSE(fds[i]);
>>       }
>> -    cleanup();
>> -    tst_exit();
>> +    SAFE_CHDIR("..");
>>   }
>> +
>> +static struct tst_test test = {
>> +    .setup = setup,
>> +    .test_all = run,
>> +    .cleanup = cleanup,
>> +    .mntpoint = MNTPOINT,
>> +    .all_filesystems = 1
> We need .needs_root if we acquire a device.

Good catch, I'll fix it in v2.

>> +};
> 
> The rest looks ok, but I'm wondering if we really need to call 
> SAFE_CHDIR() so many times before SAFE_OPEN(".", ...) instead of passing 
> the full path. We always obtain a file descriptor and in general we 
> would like to reduce calls which are not 100% needed by the tests.

Feel free to submit a follow-up patch. But the API rewrite patch itself 
should not change the original test scenario too much.

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

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

[LTP] [RFC] .all_filesystems imply .needs_root? [was Re: [PATCH] open14: Rewrite to new API]

[Petr Vorel]

Hi all,

...
> > +static struct tst_test test = {
> > +	.setup = setup,
> > +	.test_all = run,
> > +	.cleanup = cleanup,
> > +	.mntpoint = MNTPOINT,
> > +	.all_filesystems = 1
> We need .needs_root if we acquire a device.

We LTP library allows to run test under non-root user with rw permissions to
/dev/loop-control:

tst_device.c:110: TINFO: Not allowed to open /dev/loop-control. Are you root?: EACCES (13)
tst_device.c:149: TINFO: No free devices found
tst_device.c:362: TBROK: Failed to acquire device

In openSUSE that would be to run tests with user which is in disk group.
Although this should theoretically work, root is often required for something
else:

$ open14
...
tst_supported_fs_types.c:49: TINFO: mkfs is not needed for tmpfs
tst_test.c:1938: TINFO: === Testing on ext2 ===
...
open14.c:141: TINFO: Closing temporary files
open14.c:146: TPASS: Multiple files test passed
open14.c:168: TINFO: Testing linked temp files access mode
open14.c:186: TFAIL: Unexpected access mode: 1755 != 7755
open14.c:186: TFAIL: Unexpected access mode: 1755 != 7755
open14.c:186: TFAIL: Unexpected access mode: 1755 != 7755
open14.c:186: TFAIL: Unexpected access mode: 1755 != 7755
open14.c:186: TFAIL: Unexpected access mode: 1755 != 7755

Obviously with root everything works:
...
open14.c:193: TPASS: File access modes are correct

Also I'm not sure if anybody would bother with configuring group for a test
user. I suppose most of testers run LTP under root.

Also, not all tests define needs_device for these tests:

$ git grep -l -e needs_device -e all_filesystems testcases/ | wc -l
121

$ git grep -l needs_root $(git grep -l -e needs_device -e all_filesystems testcases/) | wc -l
108

Therefore I wonder if we should just imply .needs_root on .all_filesystems (and
other flags). Also doc could be tweaked to show .needs_root in the test catalog
for these tests [1].

And there are other flags which effectively enforce root, e.g. .mount_device
(mount() requires CAP_SYS_ADMIN). Here IMHO implying root makes even more sense.

Kind regards,
Petr

[1] https://linux-test-project.readthedocs.io/en/latest/users/test_catalog.html

> > +};
...

> - Andrea

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