Re: [LTP] [PATCH v2 1/2] syscalls/readlinkat01: Convert to new API

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> +static struct tcase {
> +	int *fd;
> +	const char **path;
>  	const char *exp_buf;
>  	int exp_ret;
>  	int exp_errno;

The last three fields are not unused anymore.

> -} test_cases[] = {
> -	{&dir_fd, TEST_SYMLINK, TEST_FILE, sizeof(TEST_FILE)-1, 0},
> -	{&dir_fd, abspath, TEST_FILE, sizeof(TEST_FILE)-1, 0},
> -	{&fd, TEST_SYMLINK, NULL, -1, ENOTDIR},
> -	{&fd_invalid, TEST_SYMLINK, NULL, -1, EBADF},
> -	{&fd_atcwd, TEST_SYMLINK, TEST_FILE, sizeof(TEST_FILE)-1, 0},
> +} tcases[] = {
> +	{&dir_fd, &testsymlink, TEST_FILE, sizeof(TEST_FILE)-1, 0},
> +	{&dir_fd, &abspath, TEST_FILE, sizeof(TEST_FILE)-1, 0},
> +	{&file_fd, &abspath, TEST_FILE, sizeof(TEST_FILE)-1, 0},
> +	{&fd_atcwd, &testsymlink, TEST_FILE, sizeof(TEST_FILE)-1, 0},
> +	{&fd_atcwd, &abspath, TEST_FILE, sizeof(TEST_FILE)-1, 0},
>  };

Looking at readlinkat() manual page thre is a special case where the
pathname is empty string and the call operates on the dirfd, i.e.
attempts to resolve a directory symlink. Can we please add that case
as well? Can be done in follow up patch...

> -int TST_TOTAL = ARRAY_SIZE(test_cases);
> -
> -static void verify_readlinkat(struct test_case *test)
> +static void verify_readlinkat(unsigned int i)
>  {
>  	char buf[1024];
> +	struct tcase *tc = &tcases[i];
>  
>  	memset(buf, 0, sizeof(buf));
>  
> -	TEST(readlinkat(*test->dir_fd, test->path, buf, sizeof(buf)));
> -
> -	if (TEST_RETURN != test->exp_ret) {
> -		tst_resm(TFAIL | TTERRNO,
> -		         "readlinkat() returned %ld, expected %d",
> -		         TEST_RETURN, test->exp_ret);
> -		return;
> -	}
> -
> -	if (TEST_ERRNO != test->exp_errno) {
> -		tst_resm(TFAIL | TTERRNO,
> -		         "readlinkat() returned %ld, expected %d",
> -		         TEST_RETURN, test->exp_ret);
> -		return;
> -	}
> -
> -	if (test->exp_ret > 0 && strcmp(test->exp_buf, buf)) {
> -		tst_resm(TFAIL, "Unexpected buffer have '%s', expected '%s'",
> -		         buf, test->exp_buf);
> -		return;
> -	}
> -
> -	tst_resm(TPASS | TTERRNO, "readlinkat() returned %ld", TEST_RETURN);
> -}
> -
> -int main(int ac, char **av)
> -{
> -	int lc;
> -	int i;
> -
> -	tst_parse_opts(ac, av, NULL, NULL);
> -
> -	setup();
> -
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -		for (i = 0; i < TST_TOTAL; i++)
> -			verify_readlinkat(&test_cases[i]);
> -	}
> -
> -	cleanup();
> -	tst_exit();
> +	TST_EXP_POSITIVE(readlinkat(*tc->fd, *tc->path, buf, sizeof(buf)),
> +		     "readlinkat(%d, %s, %s, %ld)",
> +		     *tc->fd, *tc->path, buf, sizeof(buf));

Can we check that the buf was filled in with the right file name here as
well? We did that in the original test. I guess that we can just do:

	if (strcmp(buf, TEST_FILE))
		tst_res(TPASS, "The filename in buffer is correct");
	else
		tst_res(TFAIL, "Wrong filename in buffer '%s'", buf);

>  }
>  
>  static void setup(void)
>  {
> -	tst_tmpdir();
>  	char *tmpdir = tst_get_tmpdir();
>  
> -	snprintf(abspath, sizeof(abspath), "%s/" TEST_SYMLINK, tmpdir);
> +	abspath = tst_aprintf("%s/" TEST_SYMLINK, tmpdir);
>  	free(tmpdir);
>  
> -	fd = SAFE_OPEN(cleanup, TEST_FILE, O_CREAT, 0600);
> -	SAFE_SYMLINK(cleanup, TEST_FILE, TEST_SYMLINK);
> -	dir_fd = SAFE_OPEN(cleanup, ".", O_DIRECTORY);
> -
> -	TEST_PAUSE;
> +	file_fd = SAFE_OPEN(TEST_FILE, O_CREAT, 0600);
> +	SAFE_SYMLINK(TEST_FILE, TEST_SYMLINK);
> +	dir_fd = SAFE_OPEN(".", O_DIRECTORY);
>  }
>  
>  static void cleanup(void)
>  {
> -	if (fd > 0 && close(fd))
> -		tst_resm(TWARN | TERRNO, "Failed to close fd");
> -
> -	if (dir_fd > 0 && close(dir_fd))
> -		tst_resm(TWARN | TERRNO, "Failed to close dir_fd");
> +	if (file_fd > -1)
> +		SAFE_CLOSE(file_fd);
>  
> -	tst_rmdir();
> +	if (dir_fd > -1)
> +		SAFE_CLOSE(dir_fd);
>  }
> +
> +static struct tst_test test = {
> +	.test = verify_readlinkat,
> +	.needs_tmpdir = 1,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.bufs = (struct tst_buffers []) {
> +		{&abspath, .size = sizeof(char)},

Again, the abspath is initialized dynamically in setup() no need to
allocate it here.

> +		{&testsymlink, .str = TEST_SYMLINK},
> +		{},
> +	},
> +	.tcnt = ARRAY_SIZE(tcases),
> +};
> -- 
> 2.39.1
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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