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

[sujiaxun]

在 2021/12/14 上午12:05, 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'
> 
Thank you, I will resubmit after making changes.