Re: [LTP] [PATCH] syscalls/fcntl05: Convert into new api

["xuyang2018.jy@fujitsu.com" <xuyang2018.jy@fujitsu.com>]

Hi Cyril
> Hi!
>> +static void verify_fcntl(void)
>>   {
>> -	int lc;
>> +	/* F_GETLK will change flock.l_type to F_UNLCK, so need to reset */
>> +	flocks.l_type = F_RDLCK;
>>
>> -	tst_parse_opts(ac, av, NULL, NULL);
>> -
>> -	setup();
>> -
>> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
>> -
>> -		tst_count = 0;
>> -
>> -		flocks.l_type = F_RDLCK;
>> -		TEST(fcntl(fd, F_GETLK,&flocks));
>> -
>> -		if (TEST_RETURN == -1)
>> -			tst_resm(TFAIL | TTERRNO, "fcntl failed");
>> -		else {
>> -			tst_resm(TPASS, "fcntl returned %ld",
>> -				 TEST_RETURN);
>> -		}
>> -
>> -	}
>> -
>> -	cleanup();
>> -	tst_exit();
>> +	TST_EXP_PASS(fcntl(fd, F_GETLK,&flocks), "fcntl(%d, F_GETLK,&flocks)", fd);
>
> Can we also check that the l_type was actually set and that the rest of
> the fields have not changed?
>
> Should be as easy as:
>
>          TST_EXP_EQ_LI(flocks.l_type, F_UNLCK);
>          TST_EXP_EQ_LI(flocks.l_whence, SEEK_CUR);
>          TST_EXP_EQ_LI(flocks.l_start, 0);
>          ...
>

Of course, will do it in v2.
>
>>   }
>>
>> -void setup(void)
>> +static void setup(void)
>>   {
>> +	char fname[255];
>> +	int pid;
>>
>> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
>> -
>> -	TEST_PAUSE;
>> -
>> -	tst_tmpdir();
>> +	pid = getpid();
>>
>> -	sprintf(fname, "tfile_%d", getpid());
>> -	if ((fd = open(fname, O_RDWR | O_CREAT, 0700)) == -1)
>> -		tst_brkm(TBROK | TERRNO, cleanup, "open failed");
>> +	sprintf(fname, "testfile_%d", pid);
>> +	fd = SAFE_OPEN(fname, O_RDWR | O_CREAT, 0700);
>
> The test creates unique temporary directory so tehre is no need to
> include the pid in the filename anymore...

Yes.


>
> The rest looks good. With the two changes applied:
>
> Reviewed-by: Cyril Hrubis<chrubis@suse.cz>
>

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