Re: [LTP] [PATCH] setrlimit01: wait for all children to exit in test3

["Salvatore CRO'" <salvatore.cro@st.com>]

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.

> 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