Re: [LTP] [PATCH v2] Add utime07 test

[Avinesh Kumar <akumar@suse.de>]

Hi Andrea,

Reviewed-by: Avinesh Kumar <akumar@suse.de>
with below comments.

On Tuesday, April 23, 2024 2:57:42 PM GMT+2 Andrea Cervesato wrote:
> From: Andrea Cervesato <andrea.cervesato@suse.com>
> 
> This test has been extracted from symlink01 test and it verifies that
> utime() is working correctly on symlink() generated files.
> 
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
> Update license
> Add check for symlink as requested
> Text output is easier to read now
> 
>  runtest/smoketest                          |   2 +-
>  runtest/syscalls                           |   2 +-
>  testcases/kernel/syscalls/utime/.gitignore |   1 +
>  testcases/kernel/syscalls/utime/utime07.c  | 100 +++++++++++++++++++++
>  4 files changed, 103 insertions(+), 2 deletions(-)
>  create mode 100644 testcases/kernel/syscalls/utime/utime07.c
> 
> diff --git a/runtest/smoketest b/runtest/smoketest
> index 83eebfe7b..f6f14fd2b 100644
> --- a/runtest/smoketest
> +++ b/runtest/smoketest
> @@ -9,7 +9,7 @@ wait02 wait02
>  write01 write01
>  symlink01 symlink01
>  stat04 symlink01 -T stat04
> -utime01A symlink01 -T utime01
> +utime07 utime07
>  rename01A symlink01 -T rename01
>  splice02 splice02 -s 20
>  df01_sh df01.sh
> diff --git a/runtest/syscalls b/runtest/syscalls
> index b9dd9fec6..4d1c52221 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -1677,12 +1677,12 @@ ustat01 ustat01
>  ustat02 ustat02
>  
>  utime01 utime01
> -utime01A symlink01 -T utime01
>  utime02 utime02
>  utime03 utime03
>  utime04 utime04
>  utime05 utime05
>  utime06 utime06
> +utime07 utime07
>  
>  utimes01 utimes01
>  
> diff --git a/testcases/kernel/syscalls/utime/.gitignore b/testcases/kernel/syscalls/utime/.gitignore
> index 94c0ae07c..403764521 100644
> --- a/testcases/kernel/syscalls/utime/.gitignore
> +++ b/testcases/kernel/syscalls/utime/.gitignore
> @@ -4,3 +4,4 @@
>  /utime04
>  /utime05
>  /utime06
> +/utime07
> diff --git a/testcases/kernel/syscalls/utime/utime07.c b/testcases/kernel/syscalls/utime/utime07.c
> new file mode 100644
> index 000000000..2647bc897
> --- /dev/null
> +++ b/testcases/kernel/syscalls/utime/utime07.c
> @@ -0,0 +1,100 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * 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 utime() is working correctly on symlink()
> + * generated files.
> + */
> +
> +#include <utime.h>
> +#include "tst_test.h"
> +
> +static void create_symlink(const char* path, const char* symname)
Looks like this is generating make check errors.
utime07.c:19: ERROR: "foo* bar" should be "foo *bar"
> +{
> +	struct stat asymlink;
> +
> +	SAFE_SYMLINK(path, symname);
> +	SAFE_LSTAT(symname, &asymlink);
> +
> +	if ((asymlink.st_mode & S_IFMT) != S_IFLNK) {
> +		tst_brk(TBROK,
> +			"symlink generated a non-symbolic link %s to %s",
> +			symname,
> +			path);
> +	}
> +}
> +
> +static void test_utime(void)
> +{
> +	char *symname = "my_symlink0";
> +	struct stat oldsym_stat;
> +	struct stat newsym_stat;
> +
> +	tst_res(TINFO, "Test if utime() changes access time");
> +
> +	create_symlink(tst_get_tmpdir(), symname);
> +	SAFE_STAT(symname, &oldsym_stat);
> +
> +	struct utimbuf utimes = {
> +		.actime = oldsym_stat.st_atime + 100,
> +		.modtime = oldsym_stat.st_mtime + 100
> +	};
> +
> +	TST_EXP_PASS(utime(symname, &utimes));
> +	SAFE_STAT(symname, &newsym_stat);
> +
> +	time_t temp, diff;
> +
> +	temp = newsym_stat.st_atime - oldsym_stat.st_atime;
> +	diff = newsym_stat.st_mtime - oldsym_stat.st_mtime - temp;

Nit: this comparison logic assumes the diff values for atime and mtime to be
same, which is the case here by using hardcoded value of 100.

Either we can define the diff value as macro, or maybe we can use

TST_EXP_EQ_LI(newsym_stat.st_atime - oldsym_stat.st_atime, 100);
TST_EXP_EQ_LI(newsym_stat.st_mtime - oldsym_stat.st_mtime, 100);

but its upto you :)

> +
> +	TST_EXP_EXPR(diff == 0,
> +		"utime() correctly changed access time");
> +
> +	SAFE_UNLINK(symname);
> +}
> +
> +static void test_utime_no_path(void)
> +{
> +	char *symname = "my_symlink1";
> +	struct utimbuf utimes;
> +
> +	tst_res(TINFO, "Test if utime() raises ENOENT when symlink points to nowhere");
> +
> +	create_symlink("bc+eFhi!k", symname);
> +	TST_EXP_FAIL(utime(symname, &utimes), ENOENT);
> +
> +	SAFE_UNLINK(symname);
> +}
> +
> +static void test_utime_loop(void)
> +{
> +	char *symname = "my_symlink2";
> +	struct utimbuf utimes;
> +
> +	tst_res(TINFO, "Test if utime() raises ELOOP when symlink is looping");
> +
> +	create_symlink(symname, symname);
> +	TST_EXP_FAIL(utime(symname, &utimes), ELOOP);
> +
> +	SAFE_UNLINK(symname);
> +}
> +
> +static void run(void)
> +{
> +	test_utime();
> +	test_utime_no_path();
> +	test_utime_loop();
> +}
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.needs_tmpdir = 1,
> +};
> 

Regards,
Avinesh



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