[PATCH v4 2/2] tests: add signal_receive.test

[Dmitry V. Levin]

On Wed, Jun 15, 2016 at 01:20:27PM +0800, Fei Jie wrote:
> * tests/signal_receive.c: New file.
> * tests/signal_receive.test: New test.
> * tests/.gitignore: Add signal_receive.
> * tests/Makefile.am (check_PROGRAMS): Likewise.
> (MISC_TESTS): Add signal_receive.test.
> ---
>  tests/.gitignore          |  1 +
>  tests/Makefile.am         |  2 ++
>  tests/signal_receive.c    | 51 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/signal_receive.test |  8 ++++++++
>  4 files changed, 62 insertions(+)
>  create mode 100644 tests/signal_receive.c
>  create mode 100755 tests/signal_receive.test
> 
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 67d0d66..72e93d8 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -231,6 +231,7 @@ shmxt
>  sigaction
>  sigaltstack
>  siginfo
> +signal_receive
>  signalfd
>  sigreturn
>  sleep
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index a136e2a..94d3d23 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -285,6 +285,7 @@ check_PROGRAMS = \
>  	sigaction \
>  	sigaltstack \
>  	siginfo \
> +	signal_receive \
>  	signalfd \
>  	sigreturn \
>  	sleep \
> @@ -662,6 +663,7 @@ MISC_TESTS = \
>  	qual_syscall.test \
>  	redirect.test \
>  	restart_syscall.test \
> +	signal_receive.test \
>  	strace-E.test \
>  	strace-S.test \
>  	strace-T.test \
> diff --git a/tests/signal_receive.c b/tests/signal_receive.c
> new file mode 100644
> index 0000000..c3027a4
> --- /dev/null
> +++ b/tests/signal_receive.c
> @@ -0,0 +1,51 @@
> +#include "tests.h"
> +#include <assert.h>
> +#include <signal.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +
> +void sig_print(const char *signame)
> +{
> +	printf("kill\\(%d, %s\\) = 0\n"
> +	       "--- %s {si_signo=%s, si_code=SI_USER, si_pid=%d,"
> +	       " si_uid=%d} ---\n",
> +	       getpid(), signame, signame, signame, getpid(), getuid());

Invoking getpid() and getuid() so many times looks odd.
Let's invoke them once and pass the result to this function.

> +}
> +
> +static void
> +handler(int sig)
> +{
> +}
> +
> +int
> +main(void)
> +{
> +	int sig;
> +	const struct sigaction act = { .sa_handler = handler };
> +	for (sig = 1; sig <= 31; sig++)
> +	{
> +		if( sig != SIGKILL && sig != SIGSTOP)
> +		{
> +			sigaction(sig, &act, NULL);
> +		}
> +	}

Opening braces in this function are not in the right place.
The tradition is to put them on the same line with the statement.

> +	sigset_t mask;
> +	sigemptyset(&mask);
> +	for (sig = 1; sig <= 31; sig++)
> +	{
> +		sigaddset(&mask, sig);
> +	}

This loop could be merged with the previous one.

> +	sigprocmask(SIG_UNBLOCK, &mask, NULL);
> +
> +	for (sig = 1; sig <= 31; sig++)
> +	{
> +		if(sig != SIGKILL && sig != SIGSTOP)
> +		{
> +			assert(kill(getpid(), sig) == 0);

I'd be very much surprised if this kill failed, but if it did,
I'd like to see the reason.  Let's use perror_msg_and_fail
in cases like this.

> +			sig_print(signal2name(sig));
> +		}
> +	}
> +
> +	return 0;
> +}
> diff --git a/tests/signal_receive.test b/tests/signal_receive.test
> new file mode 100755
> index 0000000..2e3e98d
> --- /dev/null
> +++ b/tests/signal_receive.test
> @@ -0,0 +1,8 @@
> +#!/bin/sh
> +
> +. "${srcdir=.}/init.sh"

A comment explaining what the test does would be a good addition.

> +run_prog > /dev/null
> +run_strace -a16 -esignal $args > "$EXP"
> +match_grep "$LOG" "$EXP"
> +rm -f "$EXP"

Is there any particular reason to use less precise match_grep
rather than run_strace_match_diff?


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20160615/53524550/attachment.bin>