Re: [LTP] [PATCH v2 9/9] Rewrite rename10.c using new LTP API

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

Avinesh Kumar <akumar@suse.de> writes:

> Signed-off-by: Avinesh Kumar <akumar@suse.de>
> ---
>  testcases/kernel/syscalls/rename/rename10.c | 182 +++-----------------
>  1 file changed, 22 insertions(+), 160 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/rename/rename10.c b/testcases/kernel/syscalls/rename/rename10.c
> index 4f0933320..6c14a7e28 100644
> --- a/testcases/kernel/syscalls/rename/rename10.c
> +++ b/testcases/kernel/syscalls/rename/rename10.c
> @@ -1,175 +1,37 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
>  /*
> - *
> - *   Copyright (c) International Business Machines  Corp., 2001
> - *
> - *   This program is free software;  you can redistribute it and/or modify
> - *   it under the terms of the GNU General Public License as published by
> - *   the Free Software Foundation; either version 2 of the License, or
> - *   (at your option) any later version.
> - *
> - *   This program is distributed in the hope that it will be useful,
> - *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
> - *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
> - *   the GNU General Public License for more details.
> - *
> - *   You should have received a copy of the GNU General Public License
> - *   along with this program;  if not, write to the Free Software
> - *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + * Copyright (c) International Business Machines  Corp., 2001
> + *  07/2001 Ported by Wayne Boyer
> + * Copyright (c) 2022 SUSE LLC Avinesh Kumar <avinesh.kumar@suse.com>
>   */
>  
> -/*
> - * NAME
> - *	rename10
> - *
> - * DESCRIPTION
> - *	This test will verify that rename(2) syscall fails with ENAMETOOLONG
> - *      and ENOENT
> - *
> - * ALGORITHM
> - *	Setup:
> - *		Setup signal handling.
> - *		Create temporary directory.
> - *		Pause for SIGUSR1 if option specified.
> - *              create the "old" file
> - *
> - *	Test:
> - *		Loop if the proper options are given.
> - *              1.  rename the "old" to the "new" file
> - *                  verify rename() failed with error ENAMETOOLONG
> - *
> - *              2.  "new" path contains a directory that does not exist
> - *                  rename the "old" to the "new"
> - *                  verify rename() failed with error ENOENT
> - *	Cleanup:
> - *		Print errno log and/or timing stats if options given
> - *		Delete the temporary directory created.*
> - *
> - * USAGE
> - *	rename10 [-c n] [-e] [-i n] [-I x] [-P x] [-t]
> - *	where,  -c n : Run n copies concurrently.
> - *		-e   : Turn on errno logging.
> - *		-i n : Execute test n times.
> - *		-I x : Execute test for x seconds.
> - *		-P x : Pause for x seconds between iterations.
> - *		-t   : Turn on syscall timing.
> +/*\
> + * [Description]
>   *
> - * HISTORY
> - *	07/2001 Ported by Wayne Boyer
> - *
> - * RESTRICTIONS
> - *	None.
> + * Verify that rename(2) fails with ENAMETOOLONG, when
> + * oldpath or newpath is too long.
>   */
> -#include <sys/types.h>
> -#include <fcntl.h>
> -#include <unistd.h>
> -#include <errno.h>
> -
> -#include "test.h"
> -
> -void setup();
> -void cleanup();
>  
> -char *TCID = "rename10";
> -int TST_TOTAL = 2;
> +#include <stdio.h>
> +#include "tst_test.h"
>  
> -char badmname[] =
> -    "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyz";
> +#define TEMP_FILE "tmpfile"
>  
> -int fd;
> -char fname[255], mname[255];
> -char mdir[255];
> -
> -struct test_case_t {
> -	char *fd1;
> -	char *fd2;
> -	int error;
> -} TC[] = {
> -	/* badmname is too long for a file name - ENAMETOOLONG */
> -	{
> -	fname, badmname, ENAMETOOLONG},
> -	    /* mname contains a directory component which does not exist - ENOENT */
> -	{
> -	fname, mname, ENOENT}
> -};
> +static char long_path[PATH_MAX + 2] = {[0 ... PATH_MAX + 1] = 'a'};

PATH_MAX is actually the minimum number of bytes the system is allowed
to set as the maximum path length. Meaning it can accept more. However
it seems what we are actually testing here is NAME_MAX which is only 255
and can't be bigger than 255 according to POSIX.

So I think we should be testing what happens when 256 chars are passed
as a filename (not including null terminating char).

>  
> -int main(int ac, char **av)
> +static void setup(void)
>  {
> -	int lc;
> -	int i;
> -
> -	/*
> -	 * parse standard options
> -	 */
> -	tst_parse_opts(ac, av, NULL, NULL);
> -
> -	/*
> -	 * perform global setup for test
> -	 */
> -	setup();
> -
> -	/*
> -	 * check looping state if -i option given
> -	 */
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -
> -		tst_count = 0;
> -
> -		/* loop through the test cases */
> -		for (i = 0; i < TST_TOTAL; i++) {
> -
> -			TEST(rename(TC[i].fd1, TC[i].fd2));
> -
> -			if (TEST_RETURN != -1) {
> -				tst_resm(TFAIL, "call succeeded unexpectedly");
> -				continue;
> -			}
> -
> -			if (TEST_ERRNO == TC[i].error) {
> -				tst_resm(TPASS, "expected failure - "
> -					 "errno = %d : %s", TEST_ERRNO,
> -					 strerror(TEST_ERRNO));
> -			} else {
> -				tst_resm(TFAIL, "unexpected error - %d : %s - "
> -					 "expected %d", TEST_ERRNO,
> -					 strerror(TEST_ERRNO), TC[i].error);
> -			}
> -		}
> -	}
> -
> -	cleanup();
> -	tst_exit();
> -
> +	SAFE_TOUCH(TEMP_FILE, 0700, NULL);
>  }
>  
> -/*
> - * setup() - performs all ONE TIME setup for this test.
> - */
> -void setup(void)
> +static void run(void)
>  {
> -
> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
> -
> -	TEST_PAUSE;
> -
> -	/* Create a temporary directory and make it current. */
> -	tst_tmpdir();
> -
> -	sprintf(fname, "./tfile_%d", getpid());
> -	sprintf(mdir, "./rndir_%d", getpid());
> -	sprintf(mname, "%s/rnfile_%d", mdir, getpid());
> -
> -	SAFE_TOUCH(cleanup, fname, 0700, NULL);
> +	TST_EXP_FAIL(rename(TEMP_FILE, long_path),
> +				ENAMETOOLONG);
>  }
>  
> -/*
> - * cleanup() - performs all ONE TIME cleanup for this test at
> - *             completion or premature exit.
> - */
> -void cleanup(void)
> -{
> -
> -	/*
> -	 * Remove the temporary directory.
> -	 */
> -	tst_rmdir();
> -}
> +static struct tst_test test = {
> +	.setup = setup,
> +	.test_all = run,
> +	.needs_tmpdir = 1
> +};
> -- 
> 2.36.1

Like the rest of these tests this should be run on all filesystems as
well. I guess if it fails on exFAT or something like that. Then we can
filter out those filesystems because not every FS Linux supports may be
POSIX compliant. However we probably want to make sure the main ones are
(e.g. xfs, btrfs, ext4).

-- 
Thank you,
Richard.

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