Re: [LTP] [PATCH v7] fstat: add test for multiple file types using fstat

[Petr Vorel <pvorel@suse.cz>]

Hi Jinseok,

> Following review feedback, create a dedicated fstat test that verifies
> S_ISREG(), S_ISDIR(), S_ISFIFO(), S_ISCHR(), S_ISBLK() on different file
> types.

> diff --git a/testcases/kernel/syscalls/fstat/fstat04.c b/testcases/kernel/syscalls/fstat/fstat04.c
> new file mode 100644
> index 000000000..8a52de06a
> --- /dev/null
> +++ b/testcases/kernel/syscalls/fstat/fstat04.c
> @@ -0,0 +1,103 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2026 Jinseok Kim <always.starving0@gmail.com>
> + */
> +/*\
> + * Verify that fstat correctly identifies various file types
nit: Please use :manpage:`fstat(2)` instead of just fstat.
nit: missing dot at the end.

> + */
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <sys/sysmacros.h>
> +#include "tst_test.h"
> +
> +#define MOUNT_PATH	"test_mnt"
> +#define REG_FILE	"regfile"
> +#define DIR_FILE	"dirfile"
> +#define FIFO_FILE	"fifofile"
> +#define CHR_DEV	MOUNT_PATH"/chrdev"
> +#define BLK_DEV	MOUNT_PATH"/blkdev"
> +
> +static struct tcase {
> +	const char *desc;
> +	const char *path;
> +	mode_t exp_type;
> +} tcases[] = {
> +	{ "S_IFREG",	REG_FILE,	S_IFREG },
FYI It's better to use macros to not repeat yourself ("S_IFDIR" and S_IFREG),
together with designated initializers, e.g. what is being used in e.g.
testcases/kernel/syscalls/get_mempolicy/get_mempolicy01.c.

#define DESC(x) .desc = #x, .exp_type = x

{ DESC(S_IFREG), .path = REG_FILE },

But I would prefer to drop desc entirely (useless see below).

> +	{ "S_IFDIR",	DIR_FILE,	S_IFDIR },
> +	{ "S_IFIFO",	FIFO_FILE,	S_IFIFO },
> +	{ "S_IFCHR",	CHR_DEV,	S_IFCHR },
> +	{ "S_IFBLK",	BLK_DEV,	S_IFBLK },
How about others two? At least S_IFLNK (symlink) should be simple to add,
I suppose S_IFSOCK (socket) as well.

> +};
> +
> +static void verify_fstat(unsigned int i)
> +{
> +	struct tcase *tc = &tcases[i];
> +	struct stat buf;
> +
> +	int flags = O_RDONLY | O_NONBLOCK;
> +
> +	if (tc->exp_type == S_IFDIR)
> +		flags |= O_DIRECTORY;
> +
> +	int fd = SAFE_OPEN(tc->path, flags);
> +
> +	SAFE_FSTAT(fd, &buf);
> +	SAFE_CLOSE(fd);
> +
> +	switch (tc->exp_type) {
> +	case S_IFREG:
> +		TST_EXP_EXPR(S_ISREG(buf.st_mode), "%s macro matches", tc->desc);
		TST_EXP_EXPR(S_ISREG(buf.st_mode));

Not using desc is IMHO good enough for debugging:
fstat04.c:49: TPASS: Expect: S_ISREG(buf.st_mode)

Original code did not have much of added value:

fstat04.c:49: TPASS: Expect: S_IFREG macro matches

The rest LGTM.

Kind regards,
Petr

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