[LTP] [PATCH] [v1] syscalls/symlinkat01: Convert to new API

[Cyril Hrubis]

Hi!
> -static void mysymlinkat_test(struct test_struct *desc)
> +static void run_test(unsigned int nr)
>  {
>  	int fd;
> +	struct test_case *tc = &tcases[nr];
> +
> +	setup_every_copy();
> 
> -	TEST(mysymlinkat(desc->oldfn, *desc->newfd, desc->newfn));
> -
> -	/* check return code */
> -	if (TEST_ERRNO == desc->expected_errno) {
> -		if (TEST_RETURN == 0 && desc->referencefn1 != NULL) {
> +	TEST(tst_syscall(__NR_symlinkat, tc->oldfn, *tc->newfd, tc->newfn));
> +	if (TST_ERR == tc->expected_errno) {
> +		if (TST_RET == 0 && tc->referencefn1 != NULL) {
>  			int tnum = rand(), vnum = ~tnum;
> 
> -			fd = SAFE_OPEN(cleanup, desc->referencefn1, O_RDWR);
> -			SAFE_WRITE(cleanup, 1, fd, &tnum, sizeof(tnum));
> -			SAFE_CLOSE(cleanup, fd);
> +			fd = SAFE_OPEN(tc->referencefn1, O_RDWR);
> +			SAFE_WRITE(1, fd, &tnum, sizeof(tnum));
> +			SAFE_CLOSE(fd);
> 
> -			fd = SAFE_OPEN(cleanup, desc->referencefn2, O_RDONLY);
> -			SAFE_READ(cleanup, 1, fd, &vnum, sizeof(vnum));
> -			SAFE_CLOSE(cleanup, fd);
> +			fd = SAFE_OPEN(tc->referencefn2, O_RDONLY);
> +			SAFE_READ(1, fd, &vnum, sizeof(vnum));
> +			SAFE_CLOSE(fd);
> 
>  			if (tnum == vnum)
> -				tst_resm(TPASS, "Test passed");
> +				tst_res(TPASS, "Test passed");
>  			else
> -				tst_resm(TFAIL,
> +				tst_res(TFAIL,
>  					 "The link file's content isn't as same as the original file's "
>  					 "although symlinkat returned 0");
>  		} else {
> -			tst_resm(TPASS,
> +			tst_res(TPASS,
>  				 "symlinkat() returned the expected  errno %d: %s",
> -				 TEST_ERRNO, strerror(TEST_ERRNO));
> +				 TST_ERR, strerror(TST_ERR));
>  		}

I think that the test would be much clearer to read if we split it into
two testcases, one that checks for successfull symlink creating and one
that checks for errors. Then we could easily use the TST_EXP_FAIL()
macros etc.

>  	} else {
> -		tst_resm(TFAIL,
> -			 TEST_RETURN ==
> +		tst_res(TFAIL,
> +			 TST_RET ==
>  			 0 ? "symlinkat() surprisingly succeeded" :
> -			 "symlinkat() Failed, errno=%d : %s", TEST_ERRNO,
> -			 strerror(TEST_ERRNO));
> +			 "symlinkat() Failed, errno=%d : %s", TST_ERR,
> +			 strerror(TST_ERR));
>  	}
>  }
> 
> @@ -214,27 +145,24 @@ static void setup(void)
>  {
>  	char *tmp;
>  	int fd;
> -
> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
> -
> -	tst_tmpdir();
> -
> -	SAFE_MKDIR(cleanup, TEST_DIR1, 0700);
> -	SAFE_MKDIR(cleanup, TEST_DIR3, 0700);
> -	olddirfd = SAFE_OPEN(cleanup, TEST_DIR1, O_DIRECTORY);
> -	deldirfd = SAFE_OPEN(cleanup, TEST_DIR3, O_DIRECTORY);
> -	SAFE_RMDIR(cleanup, TEST_DIR3);
> -	fd = SAFE_OPEN(cleanup, TEST_DIR1 "/" TEST_FILE1, O_CREAT | O_EXCL, 0600);
> -	SAFE_CLOSE(cleanup, fd);
> +
> +	SAFE_MKDIR(TEST_DIR1, 0700);
> +	SAFE_MKDIR(TEST_DIR3, 0700);
> +	olddirfd = SAFE_OPEN(TEST_DIR1, O_DIRECTORY);
> +	deldirfd = SAFE_OPEN(TEST_DIR3, O_DIRECTORY);
> +	SAFE_RMDIR(TEST_DIR3);
> +	fd = SAFE_OPEN(TEST_DIR1 "/" TEST_FILE1, O_CREAT | O_EXCL, 0600);
> +	SAFE_CLOSE(fd);
> 
>  	/* gratuitous memory leak here */
>  	tmp = strdup(dpathname);
>  	snprintf(dpathname, sizeof(dpathname), tmp, get_current_dir_name());

Can we fix this mess as well? There is no point actually in having the
format in a variable like this we can easily instead do:

	char *pwd;

	pwd = get_current_dir_name();
	snprintf(dpathname, sizeof(dpahtname), "%s/" TESTDIR2 "/" TEST_FILE1, pwd);
	free(pwd);

And that fixes two memory leaks we had there.

> -
> -	TEST_PAUSE;
>  }
> 
> -static void cleanup(void)
> -{
> -	tst_rmdir();
> -}
> +static struct tst_test test = {
> +    .min_kver = "2.6.16",
> +	.setup = setup,
> +    .tcnt = ARRAY_SIZE(tcases),
> +    .test = run_test,
> +    .needs_tmpdir = 1,
> +};

Other than that there are a few whitespaces wrong, see output of
'make check'

-- 
Cyril Hrubis
chrubis@suse.cz