Source-Changes-D archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: CVS commit: src




On 06.11.2016 17:38, Robert Elz wrote:
>     Date:        Sun, 6 Nov 2016 15:03:31 +0000
>     From:        "Kamil Rytarowski" <kamil%netbsd.org@localhost>
>     Message-ID:  <20161106150331.25FB2FBA6%cvs.NetBSD.org@localhost>
> 
>   | The t_wait_noproc_wnohang adds to options (except wait(2), wait3(2))
> 
> Why exclude wait3() ?   (Not that it really matters, wait3() and wait4()
> are the same thing modulo the pid paramater, which isn't being used here)
> 

This was my mistake, I will correct it.

> Could you also add tests (for wait4() and later, not wait() or wait3())
> where the process has a child, but the pid parameter (which is why not
> wait() or wait3() as there isn't one) is not the pid of the child ?
> 
> Also test (for wait3() and later) the WNOHANG case where the child
> exists, but has not exited yet ...
> 
> When you do that you also need to do a wait to clean up the child when
> it exits ... you get to also test wait returning a value, as the child
> should just sleep, then once the first waits (not expected to fetch
> the process are finished) kill() the child to avoid waiting for the
> sleep() to finish (delaying the test needlessly and perhaps getting
> caught up in the qemu timing issues) and what's more you get to check
> that the exit reason and signal number are correct (note that for waitid()
> you need to supply, and use, a siginfo_t to get the values, and should
> for wait6() as well - and verify both it, and wait6's status arg).
> 
> The basic model should be ...
> 
> 	child = fork(); /* test for -1 ... */
> 	if (child == 0) {
> 		sleep(10);	/* plenty long enough */
> 		_exit(0);
> 	}
> 	pid = waitpid(child+10, ...);		/* expect error */
> 	pid = waitpid(child, WNOHANG);	/* expect 0 */
> 	kill(child, SIGTERM);
> 	pid = waitpid(child, &status, ...);	/* expect child */
> 	/* expect WIFSIGNALLED(status), and WTERMSIG(status)==SIGTERM)
> 
> you can deal with this in one generic function to do the work,
> and N helper functions that do the waits using their different arg
> sequences - where all the helper funcs have the same arg lists (even
> down to a siginfo_t inall of them) so you can then just call the
> generic routine N times, with each of the N different wait funcs
> as an arg, like
> 
> 	int generic_func( int waitfunc(int, int, int *, siginfo_t *),
> 		 int flags, int *status, siginfo_t *si) {
> 	{
> 		/* then as above, except instead of calling waitpid()
> 		  you would do 
> 		pid = (*waitfunc)(child+10, flags, status, si); /* + checks */
> 		pid = (*waitfunc)(child, flags|WNOHANG, status, si); /* +...*/
> 		pid = (*waitfunc)(child, flags, status, si);  /* check pid==child */
> 		return pid;		/* then return as rest of checking
> 					depends on which wait func was used */
> 
> 	}
> 
> or something along those lines, and call that as
> 
> then implement
> 	waitpid_func(int child, int flags, int *status, siginfo_t *si)
> 	{
> 		return waitpid(child, status, flags);	/* si is unused here */
> 	}
> (and so on for the others), and then call
> 
> 	pid = generic_func(waitpid, WNOHANG, &status, &si);
> 	/* validate results in status - si ignored of course */
> 	pid = generic_func(waitid, WNOHANG, &status, &si);
> 	/* validate results (from si), expect 0 from final pid,
> 		 ignore unused status */
> 	/*etc etc*/
> 
> (details need to be fleshed out!)
> 
> 
> I am not yet certain that the current behaviour of wait6() and waitid()
> is actually wrong, so the expected result, and expect_fail for those ones
> might be premature.
> 
> kre
> 

Thank you for good suggestions. I will add them once t_ptrace_wait*
family of tests will land the sources.

I have also other tests in mind in the t_wait* family and I will add
them too.

I need to go out for few hours and unless there will be issues in the
code I plan to finish everything mentioned above tonight.

Attachment: signature.asc
Description: OpenPGP digital signature



Home | Main Index | Thread Index | Old Index