> > If the tests I submitted pass then I don't care whose patch are used.
Same for me. We just need the code fixed. However, my patch is a more incremental fix while Dominique's patch refactors the logic, even reducing the code size. > > +:git-shell:ec7ca9c:busybox> > ./busybox ash shell/ash_test/ash-misc/wait_n1.tests > > p1 active: 1 (expects 0) > > all passed except the above one. > > https://github.com/robang74/busybox/tree/bugfixes This is the test that failed. # Wait for any of p1, p2. p2 finishes first. (sleep 2; exit 41) & p1=$! (sleep 1; exit 42) & p2=$! wait -n $p1 $p2 echo "Wait 1 returned: $?. (expects 42)" kill -0 $p1 2>/dev/null; echo "p1 active: $? (expects 0)" kill -0 $p2 2>/dev/null; echo "p2 active: $? (expects 1)" I guess it waited for p1 and not p2. My code also handles cases like wait -n %1 %2 or wait -n $p1 $p2 using a marker (jp->wait_n). I will also add a more comprehensive test like: # Test wait -n with multiple PIDs # It should return when ANY of the specified jobs finishes. (sleep 2; exit 41) & p1=$! (sleep 1; exit 42) & p2=$! (sleep 3; exit 43) & p3=$! # Should wait for p2 (1s) and return 42 wait -n $p1 $p2 echo "First wait -n returned: $? (expects 42)" # p1 should still be running. wait -n for p1 or p3 should return when p1 finishes (after another 1s) wait -n $p1 $p3 echo "Second wait -n returned: $? (expects 41)" # wait -n for p3 (still running) wait -n $p3 echo "Third wait -n returned: $? (expects 43)" I also tested the wait8 and wait9 tests from Dominique and got a bug in my code. It was not a big code change but it is important. @@ -4927,10 +4927,7 @@ waitcmd(int argc UNUSED_PARAM, char **argv) * should wait for 2 seconds. Not 1 or 3. */ if (status != -1 && !WIFSTOPPED(status)) { - retval = WEXITSTATUS(status); - if (WIFSIGNALED(status)) - retval = 128 | WTERMSIG(status); - goto ret; + continue; } } I will also incorporate the wait8/wait9 from Dominique test as it covers more cases than my tests. I would also rename my wait_n? tests to wait9, wait10... to get them inline with the other tests, although it might get executed out of (numeric) order. I'll send a v2 with the fix and all tests. Now all tests pass. ash-misc/until1.tests: ok ash-misc/wait10.tests: ok ash-misc/wait11.tests: ok ash-misc/wait12.tests: ok ash-misc/wait4.tests: ok ash-misc/wait5.tests: ok ash-misc/wait6.tests: ok ash-misc/wait7.tests: ok ash-misc/wait8.tests: ok ash-misc/wait9.tests: ok ash-misc/while1.tests: ok Regards, Luiz _______________________________________________ busybox mailing list [email protected] https://lists.busybox.net/mailman/listinfo/busybox
