Re: [LTP] [PATCH v3] Add lstat03 test

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> 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..af852169f
> --- /dev/null
> +++ b/testcases/kernel/syscalls/lstat/lstat03.c
> @@ -0,0 +1,204 @@
> +// 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 MNTPOINT "mntpoint"
> +
> +static void test_dev(void)
> +{
> +	char *filename = "file_dev";
> +	char *symname = MNTPOINT"/sym_dev";
> +
> +	tst_res(TINFO, "Test symlink device");
> +
> +	SAFE_TOUCH(filename, 0777, NULL);
> +	SAFE_SYMLINK(filename, symname);

I suppose that I missed part of the discussion, but why isn't it
possible to create a single file that has as many things different as
possible in the test setup and simply use it in all tests?

> +	struct stat path;
> +	struct stat link;
> +
> +	TST_EXP_PASS(lstat(filename, &path));
> +	TST_EXP_PASS(lstat(symname, &link));
> +
> +	TST_EXP_EXPR(path.st_dev != link.st_dev, "path.st_dev != link.st_dev");
> +	TST_EXP_EXPR(path.st_ino != link.st_ino, "path.st_ino != link.st_ino");

Hmm, the TST_EXP_EXPR() does not seem to do the stringification as the
rest of the macros and we have to repeat the arguments here. I suppose
that we need:

diff --git a/include/tst_test_macros.h b/include/tst_test_macros.h
index 22b39fb14..5e83eeaca 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);
        }
 }


> +	SAFE_UNLINK(symname);
> +}
> +
> +static void test_nlink(void)
> +{
> +	char *filename = "file_nlink";
> +	char *symname = "sym_nlink";
> +
> +	tst_res(TINFO, "Test symlink hard link");
> +
> +	SAFE_TOUCH(filename, 0777, NULL);

We should link the filename to a different name here.

> +	SAFE_SYMLINK(filename, symname);
> +
> +	struct stat path;
> +	struct stat link;
> +
> +	TST_EXP_PASS(lstat(filename, &path));
> +	TST_EXP_PASS(lstat(symname, &link));
> +
> +	TST_EXP_EQ_LI(path.st_nlink, link.st_nlink);

And then this would be actually different, which is what we should test
for.

> +	SAFE_UNLINK(symname);
> +}
> +
> +static void test_ownership(void)
> +{
> +	char *filename = "file_all";
> +	char *symname = "sym_ownership";
> +
> +	tst_res(TINFO, "Test symlink ownership");
> +
> +	SAFE_TOUCH(filename, 0777, NULL);
> +	SAFE_SYMLINK(filename, symname);
> +
> +	SAFE_CHOWN(symname, 1000, 1000);
> +
> +	struct stat path;
> +	struct stat link;
> +
> +	TST_EXP_PASS(lstat(filename, &path));
> +	TST_EXP_PASS(lstat(symname, &link));
> +
> +	TST_EXP_EXPR(path.st_uid != link.st_uid, "path.st_uid != link.st_uid");
> +	TST_EXP_EXPR(path.st_gid != link.st_gid, "path.st_gid != link.st_gid");
> +
> +	SAFE_UNLINK(symname);
> +}
> +
> +static void test_filesize(void)
> +{
> +	char *filename = "file_size";
> +	char *symname = "sym_size";
> +	int fd;
> +
> +	tst_res(TINFO, "Test symlink filesize");
> +
> +	SAFE_TOUCH(filename, 0777, NULL);
> +
> +	fd = SAFE_OPEN(filename, O_WRONLY, 0777);
> +	tst_fill_fd(fd, 'a', TST_KB, 500);
> +	SAFE_CLOSE(fd);
> +
> +	SAFE_SYMLINK(filename, symname);
> +
> +	struct stat path;
> +	struct stat link;
> +
> +	TST_EXP_PASS(lstat(filename, &path));
> +	TST_EXP_PASS(lstat(symname, &link));
> +
> +	TST_EXP_EXPR(path.st_size != link.st_size, "path.st_size != link.st_size");
> +	TST_EXP_EXPR(path.st_blocks != link.st_blocks, "path.st_blocks != link.st_blocks");
> +
> +	SAFE_UNLINK(symname);
> +}
> +
> +static void test_blksize(void)
> +{
> +	char *filename = "file_blksize";
> +	char *symname = MNTPOINT"/sym_blksize";
> +
> +	tst_res(TINFO, "Test symlink blksize");
> +
> +	SAFE_TOUCH(filename, 0777, NULL);
> +	SAFE_SYMLINK(filename, symname);
> +
> +	struct stat path;
> +	struct stat link;
> +
> +	TST_EXP_PASS(lstat(filename, &path));
> +	TST_EXP_PASS(lstat(symname, &link));
> +
> +	TST_EXP_EXPR(path.st_blksize != link.st_blksize, "path.st_blksize != link.st_blksize");
> +
> +	SAFE_UNLINK(symname);
> +}
> +
> +static void test_timestamp(void)
> +{
> +	char *filename = "file_timestamp";
> +	char *symname = "sym_timestamp";
> +
> +	tst_res(TINFO, "Test symlink timestamp");
> +
> +	SAFE_TOUCH(filename, 0777, NULL);
> +
> +	/* we wait a bit before creating symlink in order to obtain
> +	 * different timestamp values
> +	 */
> +	sleep(1);
> +	SAFE_SYMLINK(filename, symname);

This should really be done in the test setup, since as it is now the
test would sleep for 1 second for each iteration with -i.

> +	struct stat path;
> +	struct stat link;
> +
> +	TST_EXP_PASS(lstat(filename, &path));
> +	TST_EXP_PASS(lstat(symname, &link));
> +
> +	TST_EXP_EXPR(path.st_atime != link.st_atime, "path.st_atime != link.st_atime");
> +	TST_EXP_EXPR(path.st_mtime != link.st_mtime, "path.st_mtime != link.st_mtime");
> +	TST_EXP_EXPR(path.st_ctime != link.st_ctime, "path.st_ctime != link.st_ctime");
> +
> +	SAFE_UNLINK(symname);
> +}
> +
> +static void run(void)
> +{
> +	test_dev();
> +	test_nlink();
> +	test_ownership();
> +	test_filesize();
> +	test_blksize();
> +	test_timestamp();
> +}
> +
> +static void setup(void)
> +{
> +	char opt_bsize[32];
> +	const char *const fs_opts[] = {opt_bsize, NULL};
> +	struct stat sb;
> +	int pagesize;
> +
> +	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);
> +}
> +
> +static void cleanup(void)
> +{
> +	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,
> +};
> -- 
> 2.35.3
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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