Re: [LTP] [PATCH 4/5] Add pidfd_getfd01 test

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

Hi Petr
> Hi Xu,
>
> ...
>>>> +	TEST(kcmp(getpid(), pid, KCMP_FILE, remotefd, targetfd));
>>>> +	if (TST_RET != 0)
>>>> +		tst_res(TFAIL, "pidfd_getfd() didn't get the same open file description");
>>> Maybe just:
>>>          TST_EXP_PASS_SILENT(kcmp(getpid(), pid, KCMP_FILE, remotefd, targetfd));
>> I think we can't use this macro here see below(kcmp manpage about return
>> value):
>> 0 v1 is equal to v2; in other words, the two processes share the resource.
>
>> 1   v1 is less than v2.
>
>> 2   v1 is greater than v2.
>
>> 3   v1 is not equal to v2, but ordering information is unavailable.
>
>> On error, -1 is returned, and errno is set appropriately.
>
>> So 1,2,3 are not invalid return value.
> I'm sorry, you're right.
>
>> TEST(kcmp(getpid(), pid, KCMP_FILE, remotefd, targetfd));
>> if (TST_RET==-1) {
>> 	tst_res(TFAIL | TTERRNO, "kcmp failed unexpectedly");
>> 	goto free;
>> } else {
>> 	if (TST_RET<  0 || TST_RET>  3)
> +1 (very nit: instead of if/else I'd use 2x if and (TST_RET<  -1 || TST_RET>  3)
> - readability).
Good catch. I add the last "if" otherwise 1,2,3 are ignored.

TEST(kcmp(getpid(), pid, KCMP_FILE, remotefd, targetfd));
         if (TST_RET == -1) {
                 tst_res(TFAIL | TTERRNO, "kcmp failed unexpectedly");
                 goto free;
         }
         if (TST_RET < 0 || TST_RET > 3) {
                 tst_res(TFAIL, "kcmp invalid returns value(%d)", 
(int)TST_RET);
                 goto free;
         }
         if (TST_RET != 0) {
                 tst_res(TFAIL, "kcmp returns unexpected value(%d) 
instead of 0",
                                 (int)TST_RET);
                 goto free;
         }

         tst_res(TPASS, "pidfd_getfd(%d, %d, 0) passed", pidfd, targetfd);

free:


Best Regards
Yang Xu
>
> Kind regards,
> Petr
>
>> 	tst_res(TFAIL, "kcmp invalid return value %ld", TST_RET);
>> 	goto free;
>> }
>
>> free:
>>    ....

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