[LTP] [PATCH v2 0/5] symlink01 split

[LTP] [PATCH v2 0/5] symlink01 split

[Andrea Cervesato]

This is a developement series (requested by Petr Vorel) that handle
symlink01 split, which has been already merged in the master branch.

In this series we face the next part of symlink01 split that
includes stat04, lstat03, open15 and chmod08.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
Changes in v2:
- update TST_EXP_EXTR to support stringification
- call stat() inside the test run() routine
- call lstat() inside the test run() routine
- simplify chmod08
- simplify open15
- Link to v1: https://lore.kernel.org/r/20240702-stat04-v1-0-e27d9953210d@suse.com

---
Andrea Cervesato (5):
      Add stat04 test
      Fix TST_EXP_EXTR() stringification
      Add lstat03 test
      Add chmod08 test
      Add open15 test

 include/tst_test_macros.h                  |   5 +-
 runtest/smoketest                          |   4 +-
 runtest/syscalls                           |  11 +--
 testcases/kernel/syscalls/chmod/.gitignore |   1 +
 testcases/kernel/syscalls/chmod/chmod08.c  |  45 +++++++++++
 testcases/kernel/syscalls/fork/fork04.c    |   6 +-
 testcases/kernel/syscalls/lstat/.gitignore |   2 +
 testcases/kernel/syscalls/lstat/lstat03.c  | 101 ++++++++++++++++++++++++
 testcases/kernel/syscalls/open/.gitignore  |   1 +
 testcases/kernel/syscalls/open/open15.c    |  64 +++++++++++++++
 testcases/kernel/syscalls/stat/.gitignore  |   2 +
 testcases/kernel/syscalls/stat/stat04.c    | 120 +++++++++++++++++++++++++++++
 12 files changed, 350 insertions(+), 12 deletions(-)
---
base-commit: 4dc77b84b1865b37d1023f895f176a418c06b8a7
change-id: 20240702-stat04-ceeb58b80910

Best regards,
-- 
Andrea Cervesato <andrea.cervesato@suse.com>


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

[LTP] [PATCH v2 1/5] Add stat04 test

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

This test has been extracted from symlink01 test and it checks that
stat() executed on file provide the same information of symlink linking
to it.

Reviewed-by: Li Wang <liwang@redhat.com>
Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 runtest/smoketest                         |   4 +-
 runtest/syscalls                          |   4 +-
 testcases/kernel/syscalls/stat/.gitignore |   2 +
 testcases/kernel/syscalls/stat/stat04.c   | 120 ++++++++++++++++++++++++++++++
 4 files changed, 126 insertions(+), 4 deletions(-)

diff --git a/runtest/smoketest b/runtest/smoketest
index f6f14fd2b..5608417f9 100644
--- a/runtest/smoketest
+++ b/runtest/smoketest
@@ -8,8 +8,8 @@ time01 time01
 wait02 wait02
 write01 write01
 symlink01 symlink01
-stat04 symlink01 -T stat04
-utime07 utime07
+stat04 stat04
+utime01A symlink01 -T utime01
 rename01A symlink01 -T rename01
 splice02 splice02 -s 20
 df01_sh df01.sh
diff --git a/runtest/syscalls b/runtest/syscalls
index b6cadb2df..1e1203503 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1535,8 +1535,8 @@ stat02 stat02
 stat02_64 stat02_64
 stat03 stat03
 stat03_64 stat03_64
-stat04 symlink01 -T stat04
-stat04_64 symlink01 -T stat04_64
+stat04 stat04
+stat04_64 stat04_64
 
 statfs01 statfs01
 statfs01_64 statfs01_64
diff --git a/testcases/kernel/syscalls/stat/.gitignore b/testcases/kernel/syscalls/stat/.gitignore
index fa0a4ce9f..0a62dc6ee 100644
--- a/testcases/kernel/syscalls/stat/.gitignore
+++ b/testcases/kernel/syscalls/stat/.gitignore
@@ -4,3 +4,5 @@
 /stat02_64
 /stat03
 /stat03_64
+/stat04
+/stat04_64
diff --git a/testcases/kernel/syscalls/stat/stat04.c b/testcases/kernel/syscalls/stat/stat04.c
new file mode 100644
index 000000000..4609f02d8
--- /dev/null
+++ b/testcases/kernel/syscalls/stat/stat04.c
@@ -0,0 +1,120 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
+ *    Author: David Fenner, Jon Hendrickson
+ * Copyright (C) 2024 Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * This test checks that stat() executed on file provide the same information
+ * of symlink linking to it.
+ */
+
+#include <stdlib.h>
+#include "tst_test.h"
+
+#define FILENAME "file.txt"
+#define MNTPOINT "mntpoint"
+#define SYMBNAME MNTPOINT"/file_symlink"
+
+static char symb_path[PATH_MAX];
+static char file_path[PATH_MAX];
+static struct stat *file_stat;
+static struct stat *symb_stat;
+static char *tmpdir;
+
+static void run(void)
+{
+	SAFE_STAT(file_path, file_stat);
+	SAFE_STAT(symb_path, symb_stat);
+
+	TST_EXP_EQ_LI(file_stat->st_dev, symb_stat->st_dev);
+	TST_EXP_EQ_LI(file_stat->st_mode, symb_stat->st_mode);
+	TST_EXP_EQ_LI(file_stat->st_nlink, symb_stat->st_nlink);
+	TST_EXP_EQ_LI(file_stat->st_uid, symb_stat->st_uid);
+	TST_EXP_EQ_LI(file_stat->st_gid, symb_stat->st_gid);
+	TST_EXP_EQ_LI(file_stat->st_size, symb_stat->st_size);
+	TST_EXP_EQ_LI(file_stat->st_atime, symb_stat->st_atime);
+	TST_EXP_EQ_LI(file_stat->st_mtime, symb_stat->st_mtime);
+	TST_EXP_EQ_LI(file_stat->st_ctime, symb_stat->st_ctime);
+}
+
+static void setup(void)
+{
+	char opt_bsize[32];
+	const char *const fs_opts[] = {opt_bsize, NULL};
+	struct stat sb;
+	int pagesize;
+	int fd;
+
+	tmpdir = tst_get_tmpdir();
+
+	if (strlen(tmpdir) >= (PATH_MAX - strlen(FILENAME))) {
+		tst_brk(TCONF, "Temporary folder name is too long. "
+			"Can't create file");
+	}
+
+	if (strlen(tmpdir) >= (PATH_MAX - strlen(SYMBNAME))) {
+		tst_brk(TCONF, "Temporary folder name is too long. "
+			"Can't create symbolic link");
+	}
+
+	/* change st_blksize / st_dev */
+	SAFE_STAT(".", &sb);
+	pagesize = sb.st_blksize == 4096 ? 1024 : 4096;
+
+	snprintf(opt_bsize, sizeof(opt_bsize), "-b %i", pagesize);
+	SAFE_MKFS(tst_device->dev, tst_device->fs_type, fs_opts, NULL);
+	SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, 0);
+
+	SAFE_TOUCH(FILENAME, 0777, NULL);
+
+	/* change st_nlink */
+	SAFE_LINK(FILENAME, "linked_file");
+
+	/* change st_uid and st_gid */
+	SAFE_CHOWN(FILENAME, 1000, 1000);
+
+	/* change st_size */
+	fd = SAFE_OPEN(FILENAME, O_WRONLY, 0777);
+	tst_fill_fd(fd, 'a', TST_KB, 500);
+	SAFE_CLOSE(fd);
+
+	/* change st_atime / st_mtime / st_ctime */
+	usleep(1001000);
+
+	memset(file_path, 0, PATH_MAX);
+	snprintf(file_path, PATH_MAX, "%s/%s", tmpdir, FILENAME);
+
+	memset(symb_path, 0, PATH_MAX);
+	snprintf(symb_path, PATH_MAX, "%s/%s", tmpdir, SYMBNAME);
+
+	SAFE_SYMLINK(file_path, symb_path);
+}
+
+static void cleanup(void)
+{
+	free(tmpdir);
+
+	SAFE_UNLINK(SYMBNAME);
+
+	if (tst_is_mounted(MNTPOINT))
+		SAFE_UMOUNT(MNTPOINT);
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = run,
+	.needs_root = 1,
+	.needs_tmpdir = 1,
+	.needs_device = 1,
+	.mntpoint = MNTPOINT,
+	.bufs = (struct tst_buffers []) {
+		{&file_stat, .size = sizeof(struct stat)},
+		{&symb_stat, .size = sizeof(struct stat)},
+		{}
+	}
+};

-- 
2.43.0


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

[LTP] [PATCH v2 2/5] Fix TST_EXP_EXTR() stringification

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

Follow the TST_* macros standards when it comes to stringification of
the expressions.

Reviewed-by: Li Wang <liwang@redhat.com>
Suggested-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 include/tst_test_macros.h               | 5 +++--
 testcases/kernel/syscalls/fork/fork04.c | 6 +++---
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/tst_test_macros.h b/include/tst_test_macros.h
index 22b39fb14..7a443c803 100644
--- a/include/tst_test_macros.h
+++ b/include/tst_test_macros.h
@@ -340,8 +340,9 @@ const char *tst_errno_names(char *buf, const int *exp_errs, int exp_errs_cnt);
 			&tst_exp_err__, 1, ##__VA_ARGS__);                     \
 	} while (0)
 
-#define TST_EXP_EXPR(EXPR, FMT, ...)						\
-	tst_res_(__FILE__, __LINE__, (EXPR) ? TPASS : TFAIL, "Expect: " FMT, ##__VA_ARGS__);
+#define TST_EXP_EXPR(EXPR, ...)                                              \
+       tst_res_(__FILE__, __LINE__, (EXPR) ? TPASS : TFAIL, "Expect: "       \
+                TST_FMT_(TST_2_(dummy, ##__VA_ARGS__, #EXPR), __VA_ARGS__));
 
 #define TST_EXP_EQ_(VAL_A, SVAL_A, VAL_B, SVAL_B, TYPE, PFS) do {\
 	TYPE tst_tmp_a__ = VAL_A; \
diff --git a/testcases/kernel/syscalls/fork/fork04.c b/testcases/kernel/syscalls/fork/fork04.c
index b0c6bebe0..413cd5eb4 100644
--- a/testcases/kernel/syscalls/fork/fork04.c
+++ b/testcases/kernel/syscalls/fork/fork04.c
@@ -29,7 +29,7 @@ static void run_child(void)
 
 	TST_EXP_EXPR(strcmp(ENV_VAL0, val) == 0,
 		"%s environ variable has been inherited by the child",
-		ENV_KEY)
+		ENV_KEY);
 
 	tst_res(TINFO, "Unset %s environ variable inside child", ENV_KEY);
 
@@ -72,7 +72,7 @@ static void run(void)
 	} else {
 		TST_EXP_EXPR(strcmp(ENV_VAL0, val) == 0,
 			"%s environ variable is still present inside parent",
-			ENV_KEY)
+			ENV_KEY);
 	}
 
 	TST_CHECKPOINT_WAKE_AND_WAIT(0);
@@ -85,7 +85,7 @@ static void run(void)
 	else {
 		TST_EXP_EXPR(strcmp(ENV_VAL0, val) == 0,
 			"%s environ variable didn't change inside parent",
-			ENV_KEY)
+			ENV_KEY);
 	}
 }
 

-- 
2.43.0


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

[LTP] [PATCH v2 3/5] Add lstat03 test

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

This test has been extracted from symlink01 test and it checks that
lstat() provides the right information, according with device, access
time, block size, ownership, etc.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 runtest/syscalls                           |   4 +-
 testcases/kernel/syscalls/lstat/.gitignore |   2 +
 testcases/kernel/syscalls/lstat/lstat03.c  | 101 +++++++++++++++++++++++++++++
 3 files changed, 105 insertions(+), 2 deletions(-)

diff --git a/runtest/syscalls b/runtest/syscalls
index 1e1203503..160725893 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -725,12 +725,12 @@ lseek02 lseek02
 lseek07 lseek07
 lseek11 lseek11
 
-lstat01A symlink01 -T lstat01
-lstat01A_64 symlink01 -T lstat01_64
 lstat01 lstat01
 lstat01_64 lstat01_64
 lstat02 lstat02
 lstat02_64 lstat02_64
+lstat03 lstat03
+lstat03_64 lstat03_64
 
 mallinfo02 mallinfo02
 
diff --git a/testcases/kernel/syscalls/lstat/.gitignore b/testcases/kernel/syscalls/lstat/.gitignore
index a497a445f..72cba871f 100644
--- a/testcases/kernel/syscalls/lstat/.gitignore
+++ b/testcases/kernel/syscalls/lstat/.gitignore
@@ -2,3 +2,5 @@
 /lstat01_64
 /lstat02
 /lstat02_64
+/lstat03
+/lstat03_64
diff --git a/testcases/kernel/syscalls/lstat/lstat03.c b/testcases/kernel/syscalls/lstat/lstat03.c
new file mode 100644
index 000000000..b13fe9d80
--- /dev/null
+++ b/testcases/kernel/syscalls/lstat/lstat03.c
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
+ *    Author: David Fenner, Jon Hendrickson
+ * Copyright (C) 2024 Andrea Cervesato andrea.cervesato@suse.com
+ */
+
+/*\
+ * [Description]
+ *
+ * This test verifies that lstat() provides correct information according
+ * with device, access time, block size, ownership, etc.
+ * The implementation provides a set of tests which are specific for each one
+ * of the `struct stat` used to read file and symlink information.
+ */
+
+#include "tst_test.h"
+
+#define FILENAME "file.txt"
+#define MNTPOINT "mntpoint"
+#define SYMBNAME MNTPOINT"/file_symlink"
+
+static struct stat *file_stat;
+static struct stat *symb_stat;
+
+static void run(void)
+{
+	SAFE_LSTAT(FILENAME, file_stat);
+	SAFE_LSTAT(SYMBNAME, symb_stat);
+
+	TST_EXP_EXPR(file_stat->st_dev != symb_stat->st_dev);
+	TST_EXP_EXPR(file_stat->st_mode != symb_stat->st_mode);
+	TST_EXP_EXPR(file_stat->st_nlink != symb_stat->st_nlink);
+	TST_EXP_EXPR(file_stat->st_ino != symb_stat->st_ino);
+	TST_EXP_EXPR(file_stat->st_uid != symb_stat->st_uid);
+	TST_EXP_EXPR(file_stat->st_gid != symb_stat->st_gid);
+	TST_EXP_EXPR(file_stat->st_size != symb_stat->st_size);
+	TST_EXP_EXPR(file_stat->st_blocks != symb_stat->st_blocks);
+	TST_EXP_EXPR(file_stat->st_blksize != symb_stat->st_blksize);
+	TST_EXP_EXPR(file_stat->st_atime != symb_stat->st_atime);
+	TST_EXP_EXPR(file_stat->st_mtime != symb_stat->st_mtime);
+	TST_EXP_EXPR(file_stat->st_ctime != symb_stat->st_ctime);
+}
+
+static void setup(void)
+{
+	char opt_bsize[32];
+	const char *const fs_opts[] = {opt_bsize, NULL};
+	struct stat sb;
+	int pagesize;
+	int fd;
+
+	/* change st_blksize / st_dev */
+	SAFE_STAT(".", &sb);
+	pagesize = sb.st_blksize == 4096 ? 1024 : 4096;
+
+	snprintf(opt_bsize, sizeof(opt_bsize), "-b %i", pagesize);
+	SAFE_MKFS(tst_device->dev, tst_device->fs_type, fs_opts, NULL);
+	SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, 0);
+
+	SAFE_TOUCH(FILENAME, 0777, NULL);
+
+	/* change st_nlink */
+	SAFE_LINK(FILENAME, "linked_file");
+
+	/* change st_uid and st_gid */
+	SAFE_CHOWN(FILENAME, 1000, 1000);
+
+	/* change st_size */
+	fd = SAFE_OPEN(FILENAME, O_WRONLY, 0777);
+	tst_fill_fd(fd, 'a', TST_KB, 500);
+	SAFE_CLOSE(fd);
+
+	/* change st_atime / st_mtime / st_ctime */
+	usleep(1001000);
+
+	SAFE_SYMLINK(FILENAME, SYMBNAME);
+}
+
+static void cleanup(void)
+{
+	SAFE_UNLINK(SYMBNAME);
+
+	if (tst_is_mounted(MNTPOINT))
+		SAFE_UMOUNT(MNTPOINT);
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = run,
+	.needs_root = 1,
+	.needs_tmpdir = 1,
+	.needs_device = 1,
+	.mntpoint = MNTPOINT,
+	.bufs = (struct tst_buffers []) {
+		{&file_stat, .size = sizeof(struct stat)},
+		{&symb_stat, .size = sizeof(struct stat)},
+		{}
+	}
+};

-- 
2.43.0


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

[LTP] [PATCH v2 4/5] Add chmod08 test

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

This test has been extracted from symlink01 and it verifies that
chmod() is working correctly on symlink() generated files.

Reviewed-by: Li Wang <liwang@redhat.com>
Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 runtest/syscalls                           |  1 +
 testcases/kernel/syscalls/chmod/.gitignore |  1 +
 testcases/kernel/syscalls/chmod/chmod08.c  | 45 ++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+)

diff --git a/runtest/syscalls b/runtest/syscalls
index 160725893..40c0dd070 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -72,6 +72,7 @@ chmod03 chmod03
 chmod05 chmod05
 chmod06 chmod06
 chmod07 chmod07
+chmod08 chmod08
 
 chown01 chown01
 chown01_16 chown01_16
diff --git a/testcases/kernel/syscalls/chmod/.gitignore b/testcases/kernel/syscalls/chmod/.gitignore
index 27ddfce16..f295f4dcb 100644
--- a/testcases/kernel/syscalls/chmod/.gitignore
+++ b/testcases/kernel/syscalls/chmod/.gitignore
@@ -3,3 +3,4 @@
 /chmod05
 /chmod06
 /chmod07
+/chmod08
diff --git a/testcases/kernel/syscalls/chmod/chmod08.c b/testcases/kernel/syscalls/chmod/chmod08.c
new file mode 100644
index 000000000..87519dbe8
--- /dev/null
+++ b/testcases/kernel/syscalls/chmod/chmod08.c
@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
+ *    Author: David Fenner
+ *    Copilot: Jon Hendrickson
+ * Copyright (C) 2024 Andrea Cervesato andrea.cervesato@suse.com
+ */
+
+/*\
+ * [Description]
+ *
+ * This test verifies that chmod() is working correctly on symlink()
+ * generated files.
+ */
+
+#include "tst_test.h"
+
+#define PERMS 01777
+#define TESTFILE "myobject"
+#define	SYMBNAME "my_symlink0"
+
+static void run(void)
+{
+	struct stat oldsym_stat;
+	struct stat newsym_stat;
+
+	SAFE_TOUCH(TESTFILE, 0644, NULL);
+	SAFE_SYMLINK(TESTFILE, SYMBNAME);
+	SAFE_STAT(SYMBNAME, &oldsym_stat);
+
+	TST_EXP_PASS(chmod(SYMBNAME, PERMS));
+	SAFE_STAT(SYMBNAME, &newsym_stat);
+
+	TST_EXP_EQ_LI(newsym_stat.st_mode & PERMS, PERMS);
+	TST_EXP_EXPR(oldsym_stat.st_mode != newsym_stat.st_mode,
+		"file mode has changed");
+
+	SAFE_UNLINK(SYMBNAME);
+	SAFE_UNLINK(TESTFILE);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.needs_tmpdir = 1,
+};

-- 
2.43.0


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

[LTP] [PATCH v2 5/5] Add open15 test

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

This test has been extracted from symlink01 and it verifies that
open() is working correctly on symlink() generated files.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 runtest/syscalls                          |  2 +-
 testcases/kernel/syscalls/open/.gitignore |  1 +
 testcases/kernel/syscalls/open/open15.c   | 64 +++++++++++++++++++++++++++++++
 3 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/runtest/syscalls b/runtest/syscalls
index 40c0dd070..4dfdf3782 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -923,7 +923,6 @@ nice04 nice04
 nice05 nice05
 
 open01 open01
-open01A symlink01 -T open01
 open02 open02
 open03 open03
 open04 open04
@@ -936,6 +935,7 @@ open11 open11
 open12 open12
 open13 open13
 open14 open14
+open15 open15
 
 openat01 openat01
 openat02 openat02
diff --git a/testcases/kernel/syscalls/open/.gitignore b/testcases/kernel/syscalls/open/.gitignore
index 001d874d6..af5997572 100644
--- a/testcases/kernel/syscalls/open/.gitignore
+++ b/testcases/kernel/syscalls/open/.gitignore
@@ -12,3 +12,4 @@
 /open12_child
 /open13
 /open14
+/open15
diff --git a/testcases/kernel/syscalls/open/open15.c b/testcases/kernel/syscalls/open/open15.c
new file mode 100644
index 000000000..4ad7292ae
--- /dev/null
+++ b/testcases/kernel/syscalls/open/open15.c
@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
+ *    Author: David Fenner
+ *    Copilot: Jon Hendrickson
+ * Copyright (C) 2024 Andrea Cervesato andrea.cervesato@suse.com
+ */
+
+/*\
+ * [Description]
+ *
+ * This test verifies that open() is working correctly on symlink()
+ * generated files.
+ */
+
+#include "tst_test.h"
+
+#define FILENAME "myfile.txt"
+#define SYMBNAME "myfile_symlink"
+#define BIG_STRING "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz"
+
+static char buff_file[128];
+static char buff_symlink[128];
+static int str_size;
+
+static void run(void)
+{
+	int fd_file, fd_symlink;
+
+	memset(buff_file, 0, sizeof(buff_file));
+	memset(buff_symlink, 0, sizeof(buff_symlink));
+
+	fd_file = SAFE_OPEN(FILENAME, O_CREAT | O_RDWR, 0777);
+	SAFE_WRITE(SAFE_WRITE_ALL, fd_file, BIG_STRING, str_size);
+
+	SAFE_SYMLINK(FILENAME, SYMBNAME);
+
+	SAFE_LSEEK(fd_file, 0, 0);
+	SAFE_READ(1, fd_file, buff_file, str_size);
+
+	fd_symlink = SAFE_OPEN(SYMBNAME, O_RDWR, 0777);
+	SAFE_LSEEK(fd_symlink, 0, 0);
+	SAFE_READ(1, fd_symlink, buff_symlink, str_size);
+
+	TST_EXP_EXPR(!strncmp(buff_file, buff_symlink, str_size),
+		"file data is the equivalent to symlink generated file data");
+
+	SAFE_CLOSE(fd_file);
+	SAFE_CLOSE(fd_symlink);
+
+	SAFE_UNLINK(SYMBNAME);
+	SAFE_UNLINK(FILENAME);
+}
+
+static void setup(void)
+{
+	str_size = strlen(BIG_STRING);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.needs_tmpdir = 1,
+};

-- 
2.43.0


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

Re: [LTP] [PATCH v2 3/5] Add lstat03 test

[Li Wang]

Reviewed-by: Li Wang <liwang@redhat.com>

-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH v2 5/5] Add open15 test

[Li Wang]

Hi Andrea,

On Tue, Jul 9, 2024 at 6:47 PM Andrea Cervesato <andrea.cervesato@suse.de>
wrote:

> From: Andrea Cervesato <andrea.cervesato@suse.com>
>
> This test has been extracted from symlink01 and it verifies that
> open() is working correctly on symlink() generated files.
>
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
>  runtest/syscalls                          |  2 +-
>  testcases/kernel/syscalls/open/.gitignore |  1 +
>  testcases/kernel/syscalls/open/open15.c   | 64
> +++++++++++++++++++++++++++++++
>  3 files changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 40c0dd070..4dfdf3782 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -923,7 +923,6 @@ nice04 nice04
>  nice05 nice05
>
>  open01 open01
> -open01A symlink01 -T open01
>  open02 open02
>  open03 open03
>  open04 open04
> @@ -936,6 +935,7 @@ open11 open11
>  open12 open12
>  open13 open13
>  open14 open14
> +open15 open15
>
>  openat01 openat01
>  openat02 openat02
> diff --git a/testcases/kernel/syscalls/open/.gitignore
> b/testcases/kernel/syscalls/open/.gitignore
> index 001d874d6..af5997572 100644
> --- a/testcases/kernel/syscalls/open/.gitignore
> +++ b/testcases/kernel/syscalls/open/.gitignore
> @@ -12,3 +12,4 @@
>  /open12_child
>  /open13
>  /open14
> +/open15
> diff --git a/testcases/kernel/syscalls/open/open15.c
> b/testcases/kernel/syscalls/open/open15.c
> new file mode 100644
> index 000000000..4ad7292ae
> --- /dev/null
> +++ b/testcases/kernel/syscalls/open/open15.c
> @@ -0,0 +1,64 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
> + *    Author: David Fenner
> + *    Copilot: Jon Hendrickson
> + * Copyright (C) 2024 Andrea Cervesato andrea.cervesato@suse.com
> + */
> +
> +/*\
> + * [Description]
> + *
> + * This test verifies that open() is working correctly on symlink()
> + * generated files.
> + */
> +
> +#include "tst_test.h"
> +
> +#define FILENAME "myfile.txt"
> +#define SYMBNAME "myfile_symlink"
> +#define BIG_STRING "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz"
> +
> +static char buff_file[128];
> +static char buff_symlink[128];
> +static int str_size;
> +
> +static void run(void)
> +{
> +       int fd_file, fd_symlink;
> +
> +       memset(buff_file, 0, sizeof(buff_file));
> +       memset(buff_symlink, 0, sizeof(buff_symlink));
> +
> +       fd_file = SAFE_OPEN(FILENAME, O_CREAT | O_RDWR, 0777);
>

My initial suggestion in the last email was to generate file data by
symlink, which matches the description in code comments.

    fd_symlink = SAFE_OPEN(symname, O_CREAT | O_RDWR, 0777);
    SAFE_WRITE(SAFE_WRITE_ALL, fd_symlink, BIG_STRING, str_size);

But not to create&write files via fd_file. Otherwise it deviates from the
test goal.



> +       SAFE_WRITE(SAFE_WRITE_ALL, fd_file, BIG_STRING, str_size);
> +
> +       SAFE_SYMLINK(FILENAME, SYMBNAME);
> +
> +       SAFE_LSEEK(fd_file, 0, 0);
> +       SAFE_READ(1, fd_file, buff_file, str_size);
> +
> +       fd_symlink = SAFE_OPEN(SYMBNAME, O_RDWR, 0777);
> +       SAFE_LSEEK(fd_symlink, 0, 0);
> +       SAFE_READ(1, fd_symlink, buff_symlink, str_size);
> +
> +       TST_EXP_EXPR(!strncmp(buff_file, buff_symlink, str_size),
> +               "file data is the equivalent to symlink generated file
> data");
> +
> +       SAFE_CLOSE(fd_file);
> +       SAFE_CLOSE(fd_symlink);
> +
> +       SAFE_UNLINK(SYMBNAME);
> +       SAFE_UNLINK(FILENAME);
> +}
> +
> +static void setup(void)
> +{
> +       str_size = strlen(BIG_STRING);
> +}
> +
> +static struct tst_test test = {
> +       .test_all = run,
> +       .setup = setup,
> +       .needs_tmpdir = 1,
> +};
>
> --
> 2.43.0
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>

-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH v2 2/5] Fix TST_EXP_EXTR() stringification

[Petr Vorel]

Hi all,

merged this commit (obvious fix).
Thanks!

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 2/5] Fix TST_EXP_EXTR() stringification

[Li Wang]

Petr Vorel <pvorel@suse.cz> wrote:

merged this commit (obvious fix).
> Thanks!
>

Thank you Petr.

I think it's safe to merge 1/5 to 4/5, but yes we can wait for more
comments if you like.
(Only 5/5 has a tiny issue I pointed out there)


-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH v2 1/5] Add stat04 test

[Petr Vorel]

Hi all,

> From: Andrea Cervesato <andrea.cervesato@suse.com>

> This test has been extracted from symlink01 test and it checks that
> stat() executed on file provide the same information of symlink linking
> to it.

> Reviewed-by: Li Wang <liwang@redhat.com>
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
>  runtest/smoketest                         |   4 +-
>  runtest/syscalls                          |   4 +-
>  testcases/kernel/syscalls/stat/.gitignore |   2 +
>  testcases/kernel/syscalls/stat/stat04.c   | 120 ++++++++++++++++++++++++++++++
>  4 files changed, 126 insertions(+), 4 deletions(-)

> diff --git a/runtest/smoketest b/runtest/smoketest
> index f6f14fd2b..5608417f9 100644
> --- a/runtest/smoketest
> +++ b/runtest/smoketest
> @@ -8,8 +8,8 @@ time01 time01
>  wait02 wait02
>  write01 write01
>  symlink01 symlink01
> -stat04 symlink01 -T stat04
> -utime07 utime07
> +stat04 stat04
> +utime01A symlink01 -T utime01
nit: Why replace utime07 with utime01? I suggest to merge without this change
(modify only stat04).

Original test also tested ENOENT and ELOOP, but we have this in stat03.c.
(you probably have been discussing this previously.)

@Cyril: will you add your RBT (you reviewed v1).

>  rename01A symlink01 -T rename01
>  splice02 splice02 -s 20
>  df01_sh df01.sh
> diff --git a/runtest/syscalls b/runtest/syscalls
> index b6cadb2df..1e1203503 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -1535,8 +1535,8 @@ stat02 stat02
>  stat02_64 stat02_64
>  stat03 stat03
>  stat03_64 stat03_64
> -stat04 symlink01 -T stat04
> -stat04_64 symlink01 -T stat04_64
> +stat04 stat04
> +stat04_64 stat04_64

OT: Out of curiosity, I'm looking into
testcases/kernel/syscalls/utils/newer_64.mk, I have no idea why there is
utils/newer_64.h part.

>  statfs01 statfs01
>  statfs01_64 statfs01_64
> diff --git a/testcases/kernel/syscalls/stat/.gitignore b/testcases/kernel/syscalls/stat/.gitignore
> index fa0a4ce9f..0a62dc6ee 100644
> --- a/testcases/kernel/syscalls/stat/.gitignore
> +++ b/testcases/kernel/syscalls/stat/.gitignore
> @@ -4,3 +4,5 @@
>  /stat02_64
>  /stat03
>  /stat03_64
> +/stat04
> +/stat04_64
> diff --git a/testcases/kernel/syscalls/stat/stat04.c b/testcases/kernel/syscalls/stat/stat04.c
> new file mode 100644
> index 000000000..4609f02d8
> --- /dev/null
> +++ b/testcases/kernel/syscalls/stat/stat04.c
> @@ -0,0 +1,120 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later

Also, original test was GPL v2 only, but with rewrite like this I guess we can
have GPL v2+.

> +/*
> + * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
> + *    Author: David Fenner, Jon Hendrickson
> + * Copyright (C) 2024 Andrea Cervesato <andrea.cervesato@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * This test checks that stat() executed on file provide the same information
> + * of symlink linking to it.
> + */
> +
> +#include <stdlib.h>
> +#include "tst_test.h"
> +
> +#define FILENAME "file.txt"
> +#define MNTPOINT "mntpoint"
> +#define SYMBNAME MNTPOINT"/file_symlink"
> +
> +static char symb_path[PATH_MAX];
> +static char file_path[PATH_MAX];
> +static struct stat *file_stat;
> +static struct stat *symb_stat;
> +static char *tmpdir;
> +
> +static void run(void)
> +{
> +	SAFE_STAT(file_path, file_stat);
> +	SAFE_STAT(symb_path, symb_stat);
> +
> +	TST_EXP_EQ_LI(file_stat->st_dev, symb_stat->st_dev);
> +	TST_EXP_EQ_LI(file_stat->st_mode, symb_stat->st_mode);
> +	TST_EXP_EQ_LI(file_stat->st_nlink, symb_stat->st_nlink);
> +	TST_EXP_EQ_LI(file_stat->st_uid, symb_stat->st_uid);
> +	TST_EXP_EQ_LI(file_stat->st_gid, symb_stat->st_gid);
> +	TST_EXP_EQ_LI(file_stat->st_size, symb_stat->st_size);
> +	TST_EXP_EQ_LI(file_stat->st_atime, symb_stat->st_atime);
> +	TST_EXP_EQ_LI(file_stat->st_mtime, symb_stat->st_mtime);
> +	TST_EXP_EQ_LI(file_stat->st_ctime, symb_stat->st_ctime);
> +}
> +
> +static void setup(void)
> +{
> +	char opt_bsize[32];
> +	const char *const fs_opts[] = {opt_bsize, NULL};
> +	struct stat sb;
> +	int pagesize;
> +	int fd;
> +
> +	tmpdir = tst_get_tmpdir();
> +
> +	if (strlen(tmpdir) >= (PATH_MAX - strlen(FILENAME))) {
> +		tst_brk(TCONF, "Temporary folder name is too long. "
> +			"Can't create file");
> +	}
> +
> +	if (strlen(tmpdir) >= (PATH_MAX - strlen(SYMBNAME))) {
> +		tst_brk(TCONF, "Temporary folder name is too long. "
> +			"Can't create symbolic link");
> +	}

PATH_MAX is 4096, right? Is it really needed to test the length?
> +
> +	/* change st_blksize / st_dev */
> +	SAFE_STAT(".", &sb);
> +	pagesize = sb.st_blksize == 4096 ? 1024 : 4096;
> +
> +	snprintf(opt_bsize, sizeof(opt_bsize), "-b %i", pagesize);
> +	SAFE_MKFS(tst_device->dev, tst_device->fs_type, fs_opts, NULL);
> +	SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, 0);

Isn't symlink filesystem related? Shouldn't this be run on all_filesystems?
But we could not force block size change.

> +
> +	SAFE_TOUCH(FILENAME, 0777, NULL);
> +
> +	/* change st_nlink */
> +	SAFE_LINK(FILENAME, "linked_file");
> +
> +	/* change st_uid and st_gid */
> +	SAFE_CHOWN(FILENAME, 1000, 1000);
> +
> +	/* change st_size */
> +	fd = SAFE_OPEN(FILENAME, O_WRONLY, 0777);
> +	tst_fill_fd(fd, 'a', TST_KB, 500);
> +	SAFE_CLOSE(fd);
> +
> +	/* change st_atime / st_mtime / st_ctime */
> +	usleep(1001000);
> +
> +	memset(file_path, 0, PATH_MAX);
> +	snprintf(file_path, PATH_MAX, "%s/%s", tmpdir, FILENAME);
> +
> +	memset(symb_path, 0, PATH_MAX);
> +	snprintf(symb_path, PATH_MAX, "%s/%s", tmpdir, SYMBNAME);
> +
> +	SAFE_SYMLINK(file_path, symb_path);
> +}
> +
> +static void cleanup(void)
> +{
> +	free(tmpdir);
nit: I know that tst_get_tmpdir() is first thing in setup(), but I would still
guard it with if (tmpdir) (code may change later).

> +
> +	SAFE_UNLINK(SYMBNAME);
nit: Ideally this would be guarded by flag that SAFE_SYMLINK(file_path,
symb_path) got executed.

> +
> +	if (tst_is_mounted(MNTPOINT))
> +		SAFE_UMOUNT(MNTPOINT);
> +}
> +
> +static struct tst_test test = {
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.test_all = run,
> +	.needs_root = 1,
> +	.needs_tmpdir = 1,
nit: useless tag: needs_tmpdir (can be removed before merge).

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

> +	.needs_device = 1,
> +	.mntpoint = MNTPOINT,
> +	.bufs = (struct tst_buffers []) {
> +		{&file_stat, .size = sizeof(struct stat)},
> +		{&symb_stat, .size = sizeof(struct stat)},
> +		{}
> +	}
> +};

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

Re: [LTP] [PATCH v2 5/5] Add open15 test

[Andrea Cervesato via ltp]

On 7/9/24 14:14, Li Wang wrote:
> Hi Andrea,
>
> On Tue, Jul 9, 2024 at 6:47 PM Andrea Cervesato 
> <andrea.cervesato@suse.de> wrote:
>
>     From: Andrea Cervesato <andrea.cervesato@suse.com>
>
>     This test has been extracted from symlink01 and it verifies that
>     open() is working correctly on symlink() generated files.
>
>     Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
>     ---
>      runtest/syscalls                          |  2 +-
>      testcases/kernel/syscalls/open/.gitignore |  1 +
>      testcases/kernel/syscalls/open/open15.c   | 64
>     +++++++++++++++++++++++++++++++
>      3 files changed, 66 insertions(+), 1 deletion(-)
>
>     diff --git a/runtest/syscalls b/runtest/syscalls
>     index 40c0dd070..4dfdf3782 100644
>     --- a/runtest/syscalls
>     +++ b/runtest/syscalls
>     @@ -923,7 +923,6 @@ nice04 nice04
>      nice05 nice05
>
>      open01 open01
>     -open01A symlink01 -T open01
>      open02 open02
>      open03 open03
>      open04 open04
>     @@ -936,6 +935,7 @@ open11 open11
>      open12 open12
>      open13 open13
>      open14 open14
>     +open15 open15
>
>      openat01 openat01
>      openat02 openat02
>     diff --git a/testcases/kernel/syscalls/open/.gitignore
>     b/testcases/kernel/syscalls/open/.gitignore
>     index 001d874d6..af5997572 100644
>     --- a/testcases/kernel/syscalls/open/.gitignore
>     +++ b/testcases/kernel/syscalls/open/.gitignore
>     @@ -12,3 +12,4 @@
>      /open12_child
>      /open13
>      /open14
>     +/open15
>     diff --git a/testcases/kernel/syscalls/open/open15.c
>     b/testcases/kernel/syscalls/open/open15.c
>     new file mode 100644
>     index 000000000..4ad7292ae
>     --- /dev/null
>     +++ b/testcases/kernel/syscalls/open/open15.c
>     @@ -0,0 +1,64 @@
>     +// SPDX-License-Identifier: GPL-2.0-or-later
>     +/*
>     + * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
>     + *    Author: David Fenner
>     + *    Copilot: Jon Hendrickson
>     + * Copyright (C) 2024 Andrea Cervesato andrea.cervesato@suse.com
>     + */
>     +
>     +/*\
>     + * [Description]
>     + *
>     + * This test verifies that open() is working correctly on symlink()
>     + * generated files.
>     + */
>     +
>     +#include "tst_test.h"
>     +
>     +#define FILENAME "myfile.txt"
>     +#define SYMBNAME "myfile_symlink"
>     +#define BIG_STRING
>     "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz"
>     +
>     +static char buff_file[128];
>     +static char buff_symlink[128];
>     +static int str_size;
>     +
>     +static void run(void)
>     +{
>     +       int fd_file, fd_symlink;
>     +
>     +       memset(buff_file, 0, sizeof(buff_file));
>     +       memset(buff_symlink, 0, sizeof(buff_symlink));
>     +
>     +       fd_file = SAFE_OPEN(FILENAME, O_CREAT | O_RDWR, 0777);
>
>
> My initial suggestion in the last email was to generate file data by
> symlink, which matches the description in code comments.
>
> fd_symlink = SAFE_OPEN(symname, O_CREAT | O_RDWR, 0777);
>     SAFE_WRITE(SAFE_WRITE_ALL, fd_symlink, BIG_STRING, str_size);
>
> But not to create&write files via fd_file. Otherwise it deviates from 
> the test goal.
>
Ah right, gonna fix that. Thanks
>
>     +       SAFE_WRITE(SAFE_WRITE_ALL, fd_file, BIG_STRING, str_size);
>     +
>     +       SAFE_SYMLINK(FILENAME, SYMBNAME);
>     +
>     +       SAFE_LSEEK(fd_file, 0, 0);
>     +       SAFE_READ(1, fd_file, buff_file, str_size);
>     +
>     +       fd_symlink = SAFE_OPEN(SYMBNAME, O_RDWR, 0777);
>     +       SAFE_LSEEK(fd_symlink, 0, 0);
>     +       SAFE_READ(1, fd_symlink, buff_symlink, str_size);
>     +
>     +       TST_EXP_EXPR(!strncmp(buff_file, buff_symlink, str_size),
>     +               "file data is the equivalent to symlink generated
>     file data");
>     +
>     +       SAFE_CLOSE(fd_file);
>     +       SAFE_CLOSE(fd_symlink);
>     +
>     +       SAFE_UNLINK(SYMBNAME);
>     +       SAFE_UNLINK(FILENAME);
>     +}
>     +
>     +static void setup(void)
>     +{
>     +       str_size = strlen(BIG_STRING);
>     +}
>     +
>     +static struct tst_test test = {
>     +       .test_all = run,
>     +       .setup = setup,
>     +       .needs_tmpdir = 1,
>     +};
>
>     -- 
>     2.43.0
>
>
>     -- 
>     Mailing list info: https://lists.linux.it/listinfo/ltp
>
>
>
> -- 
> Regards,
> Li Wang


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

Re: [LTP] [PATCH v2 1/5] Add stat04 test

[Andrea Cervesato via ltp]

Hi,

On 7/9/24 23:13, Petr Vorel wrote:
> Hi all,
>
>> From: Andrea Cervesato <andrea.cervesato@suse.com>
>> This test has been extracted from symlink01 test and it checks that
>> stat() executed on file provide the same information of symlink linking
>> to it.
>> Reviewed-by: Li Wang <liwang@redhat.com>
>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
>> ---
>>   runtest/smoketest                         |   4 +-
>>   runtest/syscalls                          |   4 +-
>>   testcases/kernel/syscalls/stat/.gitignore |   2 +
>>   testcases/kernel/syscalls/stat/stat04.c   | 120 ++++++++++++++++++++++++++++++
>>   4 files changed, 126 insertions(+), 4 deletions(-)
>> diff --git a/runtest/smoketest b/runtest/smoketest
>> index f6f14fd2b..5608417f9 100644
>> --- a/runtest/smoketest
>> +++ b/runtest/smoketest
>> @@ -8,8 +8,8 @@ time01 time01
>>   wait02 wait02
>>   write01 write01
>>   symlink01 symlink01
>> -stat04 symlink01 -T stat04
>> -utime07 utime07
>> +stat04 stat04
>> +utime01A symlink01 -T utime01
> nit: Why replace utime07 with utime01? I suggest to merge without this change
> (modify only stat04).
This must be an error
> Original test also tested ENOENT and ELOOP, but we have this in stat03.c.
> (you probably have been discussing this previously.)
Yeah there's no need for that since it's tested somewhere else.
>
> @Cyril: will you add your RBT (you reviewed v1).
>
>>   rename01A symlink01 -T rename01
>>   splice02 splice02 -s 20
>>   df01_sh df01.sh
>> diff --git a/runtest/syscalls b/runtest/syscalls
>> index b6cadb2df..1e1203503 100644
>> --- a/runtest/syscalls
>> +++ b/runtest/syscalls
>> @@ -1535,8 +1535,8 @@ stat02 stat02
>>   stat02_64 stat02_64
>>   stat03 stat03
>>   stat03_64 stat03_64
>> -stat04 symlink01 -T stat04
>> -stat04_64 symlink01 -T stat04_64
>> +stat04 stat04
>> +stat04_64 stat04_64
> OT: Out of curiosity, I'm looking into
> testcases/kernel/syscalls/utils/newer_64.mk, I have no idea why there is
> utils/newer_64.h part.
no idea...
>>   statfs01 statfs01
>>   statfs01_64 statfs01_64
>> diff --git a/testcases/kernel/syscalls/stat/.gitignore b/testcases/kernel/syscalls/stat/.gitignore
>> index fa0a4ce9f..0a62dc6ee 100644
>> --- a/testcases/kernel/syscalls/stat/.gitignore
>> +++ b/testcases/kernel/syscalls/stat/.gitignore
>> @@ -4,3 +4,5 @@
>>   /stat02_64
>>   /stat03
>>   /stat03_64
>> +/stat04
>> +/stat04_64
>> diff --git a/testcases/kernel/syscalls/stat/stat04.c b/testcases/kernel/syscalls/stat/stat04.c
>> new file mode 100644
>> index 000000000..4609f02d8
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/stat/stat04.c
>> @@ -0,0 +1,120 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
> Also, original test was GPL v2 only, but with rewrite like this I guess we can
> have GPL v2+.
>
>> +/*
>> + * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
>> + *    Author: David Fenner, Jon Hendrickson
>> + * Copyright (C) 2024 Andrea Cervesato <andrea.cervesato@suse.com>
>> + */
>> +
>> +/*\
>> + * [Description]
>> + *
>> + * This test checks that stat() executed on file provide the same information
>> + * of symlink linking to it.
>> + */
>> +
>> +#include <stdlib.h>
>> +#include "tst_test.h"
>> +
>> +#define FILENAME "file.txt"
>> +#define MNTPOINT "mntpoint"
>> +#define SYMBNAME MNTPOINT"/file_symlink"
>> +
>> +static char symb_path[PATH_MAX];
>> +static char file_path[PATH_MAX];
>> +static struct stat *file_stat;
>> +static struct stat *symb_stat;
>> +static char *tmpdir;
>> +
>> +static void run(void)
>> +{
>> +	SAFE_STAT(file_path, file_stat);
>> +	SAFE_STAT(symb_path, symb_stat);
>> +
>> +	TST_EXP_EQ_LI(file_stat->st_dev, symb_stat->st_dev);
>> +	TST_EXP_EQ_LI(file_stat->st_mode, symb_stat->st_mode);
>> +	TST_EXP_EQ_LI(file_stat->st_nlink, symb_stat->st_nlink);
>> +	TST_EXP_EQ_LI(file_stat->st_uid, symb_stat->st_uid);
>> +	TST_EXP_EQ_LI(file_stat->st_gid, symb_stat->st_gid);
>> +	TST_EXP_EQ_LI(file_stat->st_size, symb_stat->st_size);
>> +	TST_EXP_EQ_LI(file_stat->st_atime, symb_stat->st_atime);
>> +	TST_EXP_EQ_LI(file_stat->st_mtime, symb_stat->st_mtime);
>> +	TST_EXP_EQ_LI(file_stat->st_ctime, symb_stat->st_ctime);
>> +}
>> +
>> +static void setup(void)
>> +{
>> +	char opt_bsize[32];
>> +	const char *const fs_opts[] = {opt_bsize, NULL};
>> +	struct stat sb;
>> +	int pagesize;
>> +	int fd;
>> +
>> +	tmpdir = tst_get_tmpdir();
>> +
>> +	if (strlen(tmpdir) >= (PATH_MAX - strlen(FILENAME))) {
>> +		tst_brk(TCONF, "Temporary folder name is too long. "
>> +			"Can't create file");
>> +	}
>> +
>> +	if (strlen(tmpdir) >= (PATH_MAX - strlen(SYMBNAME))) {
>> +		tst_brk(TCONF, "Temporary folder name is too long. "
>> +			"Can't create symbolic link");
>> +	}
> PATH_MAX is 4096, right? Is it really needed to test the length?
Yes, check previous conversation (easily buffer overflow).
>> +
>> +	/* change st_blksize / st_dev */
>> +	SAFE_STAT(".", &sb);
>> +	pagesize = sb.st_blksize == 4096 ? 1024 : 4096;
>> +
>> +	snprintf(opt_bsize, sizeof(opt_bsize), "-b %i", pagesize);
>> +	SAFE_MKFS(tst_device->dev, tst_device->fs_type, fs_opts, NULL);
>> +	SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, 0);
> Isn't symlink filesystem related? Shouldn't this be run on all_filesystems?
> But we could not force block size change.
This was suggested by @Cyril
>
>> +
>> +	SAFE_TOUCH(FILENAME, 0777, NULL);
>> +
>> +	/* change st_nlink */
>> +	SAFE_LINK(FILENAME, "linked_file");
>> +
>> +	/* change st_uid and st_gid */
>> +	SAFE_CHOWN(FILENAME, 1000, 1000);
>> +
>> +	/* change st_size */
>> +	fd = SAFE_OPEN(FILENAME, O_WRONLY, 0777);
>> +	tst_fill_fd(fd, 'a', TST_KB, 500);
>> +	SAFE_CLOSE(fd);
>> +
>> +	/* change st_atime / st_mtime / st_ctime */
>> +	usleep(1001000);
>> +
>> +	memset(file_path, 0, PATH_MAX);
>> +	snprintf(file_path, PATH_MAX, "%s/%s", tmpdir, FILENAME);
>> +
>> +	memset(symb_path, 0, PATH_MAX);
>> +	snprintf(symb_path, PATH_MAX, "%s/%s", tmpdir, SYMBNAME);
>> +
>> +	SAFE_SYMLINK(file_path, symb_path);
>> +}
>> +
>> +static void cleanup(void)
>> +{
>> +	free(tmpdir);
> nit: I know that tst_get_tmpdir() is first thing in setup(), but I would still
> guard it with if (tmpdir) (code may change later).
>
>> +
>> +	SAFE_UNLINK(SYMBNAME);
> nit: Ideally this would be guarded by flag that SAFE_SYMLINK(file_path,
> symb_path) got executed.
What if it doesn't?
>
>> +
>> +	if (tst_is_mounted(MNTPOINT))
>> +		SAFE_UMOUNT(MNTPOINT);
>> +}
>> +
>> +static struct tst_test test = {
>> +	.setup = setup,
>> +	.cleanup = cleanup,
>> +	.test_all = run,
>> +	.needs_root = 1,
>> +	.needs_tmpdir = 1,
> nit: useless tag: needs_tmpdir (can be removed before merge).
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> Kind regards,
> Petr
>
>> +	.needs_device = 1,
>> +	.mntpoint = MNTPOINT,
>> +	.bufs = (struct tst_buffers []) {
>> +		{&file_stat, .size = sizeof(struct stat)},
>> +		{&symb_stat, .size = sizeof(struct stat)},
>> +		{}
>> +	}
>> +};

Andrea


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

Re: [LTP] [PATCH v2 1/5] Add stat04 test

[Petr Vorel]

Hi Andrea,

...
> > > +static void cleanup(void)
> > > +{
> > > +	free(tmpdir);
> > nit: I know that tst_get_tmpdir() is first thing in setup(), but I would still
> > guard it with if (tmpdir) (code may change later).

> > > +
> > > +	SAFE_UNLINK(SYMBNAME);
> > nit: Ideally this would be guarded by flag that SAFE_SYMLINK(file_path,
> > symb_path) got executed.
> What if it doesn't?

Well, just one more WARN, no big deal. Just it might be misleading for a
reviewer. I see you fixed it in v3, thanks!

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 1/5] Add stat04 test

[Cyril Hrubis]

Hi!
> PATH_MAX is 4096, right? Is it really needed to test the length?

Contrary to the popular belief PATH_MAX is a limit for lenght of a
single filesytem path component, i.e. directory or a file name. And yes
I've seen bugreports for LTP where people pointed TMPDIR to a deep
enough filesystem path for this to matter.

Anyways I've proposed to allocate the paths instead with asprintf().

> > +
> > +	/* change st_blksize / st_dev */
> > +	SAFE_STAT(".", &sb);
> > +	pagesize = sb.st_blksize == 4096 ? 1024 : 4096;
> > +
> > +	snprintf(opt_bsize, sizeof(opt_bsize), "-b %i", pagesize);
> > +	SAFE_MKFS(tst_device->dev, tst_device->fs_type, fs_opts, NULL);
> > +	SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, 0);
> 
> Isn't symlink filesystem related? Shouldn't this be run on all_filesystems?

Hmm, I was assuming that symlink was a generic funcionality, but
thinking about it the symlink data has to be stored in the filesystem so
I suppose that there is some filesystem specific code to be tested after
all.

I guess that this could stil be implemented but we would have to add
support for multiple devices. I can have a look later on, but I wouldn't
block this patch because of that.

> > +
> > +	SAFE_TOUCH(FILENAME, 0777, NULL);
> > +
> > +	/* change st_nlink */
> > +	SAFE_LINK(FILENAME, "linked_file");
> > +
> > +	/* change st_uid and st_gid */
> > +	SAFE_CHOWN(FILENAME, 1000, 1000);
> > +
> > +	/* change st_size */
> > +	fd = SAFE_OPEN(FILENAME, O_WRONLY, 0777);
> > +	tst_fill_fd(fd, 'a', TST_KB, 500);
> > +	SAFE_CLOSE(fd);
> > +
> > +	/* change st_atime / st_mtime / st_ctime */
> > +	usleep(1001000);
> > +
> > +	memset(file_path, 0, PATH_MAX);
> > +	snprintf(file_path, PATH_MAX, "%s/%s", tmpdir, FILENAME);
> > +
> > +	memset(symb_path, 0, PATH_MAX);
> > +	snprintf(symb_path, PATH_MAX, "%s/%s", tmpdir, SYMBNAME);
> > +
> > +	SAFE_SYMLINK(file_path, symb_path);
> > +}
> > +
> > +static void cleanup(void)
> > +{
> > +	free(tmpdir);
> nit: I know that tst_get_tmpdir() is first thing in setup(), but I would still
> guard it with if (tmpdir) (code may change later).
> 
> > +
> > +	SAFE_UNLINK(SYMBNAME);
> nit: Ideally this would be guarded by flag that SAFE_SYMLINK(file_path,
> symb_path) got executed.

I do not think that we need to unlink the symlink, it should be removed
by the test library when the temorary directory is removed. Or is there
a problem with that?

-- 
Cyril Hrubis
chrubis@suse.cz

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