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<[email protected]> >>>> --- >>>> 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 [email protected] https://lists.sourceforge.net/lists/listinfo/ltp-list
