Re: [LTP] [PATCH v1] Rewrite process_vm01 test using new LTP API

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

Andrea Cervesato via ltp <ltp@lists.linux.it> writes:

> On 8/26/22 10:40, Andrea Cervesato via ltp wrote:
>> Hi!
>>
>> On 8/26/22 09:04, Richard Palethorpe wrote:
>>> Hello,
>>>
>>> Andrea Cervesato via ltp <ltp@lists.linux.it> writes:
>>>
>>>> -static void cma_test_invalid_perm(void)
>>>> +static void test_invalid_perm(void)
>>>>   {
>>>>       char nobody_uid[] = "nobody";
>>>>       struct passwd *ltpuser;
>>>> -    int status;
>>>>       struct process_vm_params *params;
>>>>       pid_t child_pid;
>>>>       pid_t parent_pid;
>>>> -    int ret = 0;
>>>> +    int status;
>>>> +
>>>> +    tst_res(TINFO, "Testing invalid permissions on given PID");
>>>>   -    tst_resm(TINFO, "test_invalid_perm");
>>>>       parent_pid = getpid();
>>>> -    child_pid = fork();
>>>> -    switch (child_pid) {
>>>> -    case -1:
>>>> -        tst_brkm(TBROK | TERRNO, cleanup, "fork");
>>>> -        break;
>>>> -    case 0:
>>>> -        ltpuser = getpwnam(nobody_uid);
>>>> -        if (ltpuser == NULL)
>>>> -            tst_brkm(TBROK | TERRNO, NULL, "getpwnam failed");
>>>> -        SAFE_SETUID(NULL, ltpuser->pw_uid);
>>>> -
>>>> -        params = cma_alloc_sane_params();
>>>> +    child_pid = SAFE_FORK();
>>>> +    if (!child_pid) {
>>>> +        ltpuser = SAFE_GETPWNAM(nobody_uid);
>>>> +        SAFE_SETUID(ltpuser->pw_uid);
>>>> +
>>>> +        params = alloc_params();
>>>>           params->pid = parent_pid;
>>>> -        cma_test_params(params);
>>>> -        ret |= cma_check_ret(-1, TEST_RETURN);
>>>> -        ret |= cma_check_errno(EPERM);
>>>> -        cma_free_params(params);
>>>> -        exit(ret);
>>>> -    default:
>>>> -        SAFE_WAITPID(cleanup, child_pid, &status, 0);
>>>> -        if (!WIFEXITED(status) || WEXITSTATUS(status) != 0)
>>>> -            tst_resm(TFAIL, "child returns %d", status);
>>>> +        test_params(params);
>>>> +        TST_EXP_EQ_LI(TST_RET, -1);
>>>> +        check_errno(EPERM);
>>>> +        free_params(params);
>>>> +        return;
>>>>       }
>>>> +
>>>> +    SAFE_WAITPID(child_pid, &status, 0);
>>> We want to use tst_reap_children() here which will check the exit
>>> status.
>>>
>>> In fact, if SAFE_WAITPID is removed altogether then the exit status will
>>> be checked automatically at the end of the test when the lib calls
>>> reap_children.
>>>
>>> Otherwise LGTM!
>>>
>> Isn't tst_reap_childread() already called at the end of the test_all
>> function? (tst_test.c:1354)
>>
>> Andrea
>>
>>
> The reason why I'm using SAFE_WAITPID here is that there are many
> other test functions and when test_invalid_perm is called without it,
> the tst_res messages might come out asynchronously with the other
> tests messages.

If asynchronous messages are a problem then you can explicitly call
tst_reap_children() after fork. If you use SAFE_WAITPID then you need to
check the exit status manually, it won't get checked later by the call
to tst_reap_children in tst_test.c. Because the child process has
already been reaped by waitpid.

I don't have a strong opinion on whether asynchronous messages are a
problem.

-- 
Thank you,
Richard.

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