[PATCH] tests: add test for P_PGID in waitid

Dmitry V. Levin ldv at altlinux.org
Mon Mar 14 15:56:18 UTC 2022


On Mon, Mar 14, 2022 at 06:30:44PM +0800, SuHsueyu wrote:
> ---
> GSoC micro project. This patch try to cover the P_PGID in waitid.

Great, you made the first step and found a piece of code that really
needs to be tested.

>  tests/waitid.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/tests/waitid.c b/tests/waitid.c
> index a14899530..2ca81e921 100644
> --- a/tests/waitid.c
> +++ b/tests/waitid.c
> @@ -236,10 +236,17 @@ main(void)
>  	tprintf("waitid(P_PID, %d, %s, WEXITED, %s) = 0\n",
>  		pid, sprint_siginfo(sinfo, "0"), sprint_rusage(rusage));
>  
> +	pid_t pgid = getpgid(pid);
> +	long pgrc = do_waitid(P_PGID, pgid, sinfo, WEXITED, rusage);
> +	tprintf("waitid(P_PGID, %d, %p, WEXITED, %p)"
> +		" = %ld %s (%m)\n", pgid, sinfo, rusage, pgrc, errno2name());
> +

While your change to the test technically covers "case P_PGID" in
src/wait.c, it does little to check correctness.  For example, if
"case P_PGID" contained a typo, e.g. used "printpid(tcp, id, PT_TGID)"
instead of "printpid(tcp, id, PT_PGID)", your test wouldn't catch this.

I suggest creating a new waitid-based test, e.g. waitid-Y, that would
utilize "strace -Y" to differentiate between PT_TGID and PT_PGID.

>  	long rc = do_waitid(P_ALL, -1, sinfo, WEXITED|WSTOPPED, rusage);
>  	tprintf("waitid(P_ALL, -1, %p, WEXITED|WSTOPPED, %p)"
>  		" = %ld %s (%m)\n", sinfo, rusage, rc, errno2name());
>  
> +	
> +
>  	tprintf("%s\n", "+++ exited with 0 +++");
>  	return 0;
>  }

There is no need to add extra empty lines.


-- 
ldv


More information about the Strace-devel mailing list