[LTP] [PATCH v4] Add stat04 test

[Andrea Cervesato]

Hi!

On 2/23/24 00:36, Petr Vorel wrote:
>> 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.
>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
>> ---
>> free() tmpdir
>> rename link_stat into link
>> rename path_stat into path
>>   runtest/smoketest                         |  2 +-
>>   runtest/syscalls                          |  4 +-
>>   testcases/kernel/syscalls/stat/.gitignore |  2 +
>>   testcases/kernel/syscalls/stat/stat04.c   | 50 +++++++++++++++++++++++
>>   4 files changed, 55 insertions(+), 3 deletions(-)
>>   create mode 100644 testcases/kernel/syscalls/stat/stat04.c
>> diff --git a/runtest/smoketest b/runtest/smoketest
>> index 83eebfe7b..5608417f9 100644
>> --- a/runtest/smoketest
>> +++ b/runtest/smoketest
>> @@ -8,7 +8,7 @@ time01 time01
>>   wait02 wait02
>>   write01 write01
>>   symlink01 symlink01
>> -stat04 symlink01 -T stat04
>> +stat04 stat04
>>   utime01A symlink01 -T utime01
>>   rename01A symlink01 -T rename01
>>   splice02 splice02 -s 20
>> diff --git a/runtest/syscalls b/runtest/syscalls
>> index 7794f1465..ef90076e4 100644
>> --- a/runtest/syscalls
>> +++ b/runtest/syscalls
>> @@ -1529,8 +1529,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..aebfacf5a
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/stat/stat04.c
>> @@ -0,0 +1,50 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
>> + *    Author: David Fenner
>> + *    Copilot: Jon Hendrickson
> very nit:
>   * 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"
>> +
>> +static void run(void)
>> +{
>> +	char *symname = "my_symlink0";
>> +	char *tmpdir = tst_get_tmpdir();
>> +
>> +	SAFE_SYMLINK(tmpdir, symname);
>> +
>> +	struct stat path;
>> +	struct stat link;
> nit: maybe define struct at the top of the function?
This is common in the first versions of C, but a good practice is to 
define variables as close as possible where they are used in order to 
improve readability.
>> +
>> +	TST_EXP_PASS(stat(tmpdir, &path));
>> +	free(tmpdir);
> If SAFE_SYMLINK() fails, free() will not happen. I wonder if we should bother
> (we'd have to set it NULL and add a cleanup function).
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> Kind regards,
> Petr
>
>> +
>> +	TST_EXP_PASS(stat(symname, &link));
>> +
>> +	TST_EXP_EQ_LI(path.st_dev, link.st_dev);
>> +	TST_EXP_EQ_LI(path.st_mode, link.st_mode);
>> +	TST_EXP_EQ_LI(path.st_nlink, link.st_nlink);
>> +	TST_EXP_EQ_LI(path.st_uid, link.st_uid);
>> +	TST_EXP_EQ_LI(path.st_gid, link.st_gid);
>> +	TST_EXP_EQ_LI(path.st_size, link.st_size);
>> +	TST_EXP_EQ_LI(path.st_atime, link.st_atime);
>> +	TST_EXP_EQ_LI(path.st_mtime, link.st_mtime);
>> +	TST_EXP_EQ_LI(path.st_ctime, link.st_ctime);
>> +
>> +	SAFE_UNLINK(symname);
>> +}
>> +
>> +static struct tst_test test = {
>> +	.test_all = run,
>> +	.needs_tmpdir = 1,
>> +};

According to Cyril suggestions we are probably done with this patch so 
it can be merged. Isn't it?

Thanks,
Andrea