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

[Petr Vorel <pvorel@suse.cz>]

Hi Andrea,

I was thinking if this is filesystem specific (struct inode_operations mkdir
member is in fs/*/*.c - all filesystems), but it looks to me that code which
checks for EEXIST is in may_create() in fs/namei.c, which is VFS. Therefore
there is really no point to use .all_filesystems = 1.

LGTM, just few nits below.
Reviewed-by: Petr Vorel <pvorel@suse.cz>

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

> This test has been extracted from symlink01 and it verifies that
> mkdir() can't overwrite certain types of files, such as simlinks,
                                                        ^ symlinks

> directories, pipes, devices, etc.

> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
>  runtest/syscalls                           |  2 +-
>  testcases/kernel/syscalls/mkdir/.gitignore |  1 +
>  testcases/kernel/syscalls/mkdir/mkdir10.c  | 57 ++++++++++++++++++++++
>  3 files changed, 59 insertions(+), 1 deletion(-)
>  create mode 100644 testcases/kernel/syscalls/mkdir/mkdir10.c

> diff --git a/runtest/syscalls b/runtest/syscalls
> index 2af7ade9c..7f4edb901 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -757,8 +757,8 @@ mkdir02 mkdir02
>  mkdir03 mkdir03
>  mkdir04 mkdir04
>  mkdir05 mkdir05
> -mkdir05A symlink01 -T mkdir05
This is not the case, but beware runtest/smoketest also use some of symlink
tests you recently rewrite.

>  mkdir09 mkdir09
> +mkdir10 mkdir10
...
> --- /dev/null
> +++ b/testcases/kernel/syscalls/mkdir/mkdir10.c
> @@ -0,0 +1,57 @@
> +// 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:
Authors: David Fenner, Jon Hendrickson

> + * Copyright (C) 2024 Andrea Cervesato andrea.cervesato@suse.com
> + */
> +
> +/*\
> + * [Description]
> + *
> + * This test verifies that mkdir() can't overwrite certain generated files, such
> + * as symlinks, pipes, devices, folders, etc.
> + */
> +
> +#include "tst_test.h"
> +
> +#define FILE_FOLDER "myfolder"
> +#define FILE_MYFILE "myfile"
> +#define FILE_FIFO "mypipe"
> +#define FILE_SYMLINK "mylink"
> +#define FILE_DEVICE "/dev/null"
nit: _PATH_DEVNULL from <paths.h> defines "/dev/null".
> +
> +struct tcase {
> +	char *file;
> +	char *msg;
> +};
> +
> +static struct tcase tcases[] = {
> +	{FILE_FOLDER,	"folder already exists"},
> +	{FILE_MYFILE,	"file already exists"},
> +	{FILE_FIFO,		"fifo already exists"},
> +	{FILE_SYMLINK,	"symlink already exists"},
> +	{FILE_DEVICE,	"device already exists"},
very nit: "folder", "file", ...

	TST_EXP_FAIL(mkdir(tc->file, 0777), EEXIST, "%s already exists", tc->msg);

Kind regards,
Petr

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