[prev in list] [next in list] [prev in thread] [next in thread] 

List:       ltp-list
Subject:    Re: [LTP] [PATCH] setrlimit01: wait for all children to exit in test3
From:       "Salvatore CRO'" <salvatore.cro () st ! com>
Date:       2012-03-23 7:13:11
Message-ID: 4F6C2287.9020103 () st ! com
[Download RAW message or body]

On 3/21/2012 12:10 PM, Salvatore CRO' wrote:
> On 3/21/2012 10:46 AM, Caspar Zhang wrote:
>> On 03/21/2012 05:41 PM, ZhoupingLiu wrote:
>>> On 03/20/2012 11:39 PM, Salvatore CRO' wrote:
>>>> In test3, parent just wait for one child to exit but it
>>>> actually has 10.
>>> hi, Salvatore
>>>
>>> yes, you are right, but your patch didn't resolve the issue completely.
>>> comments in-line.
>>>
>>>> As it could happen there may be other children around that
>>>> didn't get yet the chance to exit, they will get signaled
>>>> by ltp-pan (SIGTERM) and pollute the output of subsequent
>>>> test (setrlimit02).
>>>> Parent definitely needs to wait for all children to exit.
>>>>
>>>> Signed-off-by: Salvatore Cro<salvatore.cro@st.com>
>>>> ---
>>>>    testcases/kernel/syscalls/setrlimit/setrlimit01.c |    2 +-
>>>>    1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/testcases/kernel/syscalls/setrlimit/setrlimit01.c 
>>>> b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>>>> index ac32450..430835c 100644
>>>> --- a/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>>>> +++ b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>>>> @@ -267,7 +267,7 @@ void test3()
>>>>                exit(0);
>>>>            }
>>>>        }
>>>> -    waitpid(pid,&status, 0);
>>>> +    while(wait(&status)>   0) { /* no-op */ ; }
>>>>        if (WEXITSTATUS(status) != 0) {
>>>>            tst_resm(TFAIL, "RLIMIT_NPROC functionality is not 
>>>> correct");
>>>>        } else {
>>> if one child exit unexpectedly, e.g: exit(9);
>>> the case can't catch it. how about this patch set:
>
> Question is, do we care about prematurely ended child?
> We could just test for this condition ( if may use WIFEXITED in this 
> case)
> and emit a TWARN.
>
>>> diff --git a/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>>> b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>>> index ac32450..ec3e54e 100644
>>> --- a/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>>> +++ b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>>> @@ -226,6 +226,8 @@ void test2()
>>>     */
>>>    void test3()
>>>    {
>>> +       int flag = 0;
>>> +
>>>           if (getrlimit(RLIMIT_NPROC,&save_rlim)<  0) {
>>>                   tst_brkm(TBROK, cleanup, "getrlimit failed, errno: 
>>> %d",
>>> errno);
>>>           }
>>> @@ -267,12 +269,16 @@ void test3()
>>>                           exit(0);
>>>                   }
>>>           }
>>> -       waitpid(pid,&status, 0);
>>> -       if (WEXITSTATUS(status) != 0) {
>>> -               tst_resm(TFAIL, "RLIMIT_NPROC functionality is not
>>> correct");
>>> -       } else {
>>> -               tst_resm(TPASS, "RLIMIT_NPROC functionality is 
>>> correct");
>>> +       while (wait(&status)>  0) {
>>> +               if (WEXITSTATUS(status) != 0) {
>>> +                       tst_resm(TFAIL,
>>> +                           "RLIMIT_NPROC functionality is not 
>>> correct");
>>> +                       flag = 1;
>>> +               }
>>>           }
>>> +
>>> +       if (flag == 0)
>>> +               tst_resm(TPASS, "RLIMIT_NPROC functionality is 
>>> correct");
>> flag is not necessary. no TFAIL means TPASS. Rest looks good for your 
>> patch.
>
> Anyhow, in my opinion, this condition is not suitable to decide for 
> test pass/fail.
> Test is supposed to fail when the fork from 11th child on does not 
> fail or fail with
> an errno != EAGAIN.
> If so, the test logic wasn't actually accurate independently of my patch.
>
> BR,
> Salvo.
>

What about above analysis ? Any comment is welcome.

BR,
Salvo.


>> Thanks,
>> Caspar
>


------------------------------------------------------------------------------
This SF email is sponsosred by:
Try Windows Azure free for 90 days Click Here 
http://p.sf.net/sfu/sfd2d-msazure
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
[prev in list] [next in list] [prev in thread] [next in thread]