On Wed, Jul 27, 2016 at 08:41:17PM +0200, Nahim El Atmani wrote: > +# include <signal.h> > +#include <errno.h> > +#include <stdio.h> > +#include <string.h> > +# include <unistd.h>
What a strange indentation.
> +static void
> +handler(int sig)
> +{
> +}
This handler doesn't do much, why not use SIG_IGN?
> +static void
> +occ_at(int raw)
> +{
> + pid_t pid = getpid();
> + syscall(__NR_kill, pid, SIGALRM);
> + syscall(__NR_kill, pid, SIGALRM);
> + if (!raw)
> + printf("kill(%d, SIGALRM) = 0\n"
> + "kill(%d, SIGALRM) = -1 EINVAL (Invalid
> argument)(DISCARDED)\n"
This "(DISCARDED)" doesn't look quite informative. If this syscall failure
is due to fault injection, why not write something to the point, e.g.
" (fault injected)"?
> + "+++ exited with 0 +++\n", pid, pid);
> + else
> + printf("kill(%d, 0xe) = 0\n"
> + "kill(%d, 0xe) = -1 (errno 22)(DISCARDED)\n"
> + "+++ exited with 0 +++\n", pid, pid);
> +}
This alteration in expected output doesn't help to detect the case when
"at" mode is implemented as "every".
You don't have to hardcode values of SIGALRM and EINVAL.
> +static void
> +occ_every(void)
> +{
> + pid_t pid = getpid();
> + syscall(__NR_kill, pid, SIGALRM);
> + syscall(__NR_kill, pid, SIGALRM);
> + syscall(__NR_kill, pid, SIGALRM);
> + syscall(__NR_kill, pid, SIGALRM);
> + syscall(__NR_kill, pid, SIGALRM);
> + syscall(__NR_kill, pid, SIGALRM);
> + printf("kill(%d, SIGALRM) = 0\n"
> + "kill(%d, SIGALRM) = -1 EINVAL (Invalid argument)(DISCARDED)\n"
> + "kill(%d, SIGALRM) = 0\n"
> + "kill(%d, SIGALRM) = -1 EINVAL (Invalid argument)(DISCARDED)\n"
> + "kill(%d, SIGALRM) = 0\n"
> + "kill(%d, SIGALRM) = -1 EINVAL (Invalid argument)(DISCARDED)\n"
> + "+++ exited with 0 +++\n", pid, pid, pid, pid, pid, pid);
> +}
Why not just send a fixed batch of signals regardless of testing mode
and print mode specific expected output?
> +# Ensure that strace -e faultwith=<syscall>:<occurrence>:<error>,... works.
> +
> +. "${srcdir=.}/init.sh"
> +
> +run_strace -a0 -e trace=kill -e fault=kill:2:EINVAL -e signal=none ./fault
> at > "$EXP"
> +match_diff "$EXP" "$LOG"
> +run_strace -a0 -e trace=kill -e fault=kill:2.:EINVAL -e signal=none ./fault
> every > "$EXP"
> +match_diff "$EXP" "$LOG"
> +run_strace -a0 -e trace=kill -e raw=kill -e fault=kill:2.:EINVAL -e
> signal=none ./fault raw > "$EXP"
> diff --git a/tests/fault_parsing.test b/tests/fault_parsing.test
What's the purpose of the last run_strace?
You could avoid this mistake by writing a common test function
and calling it thrice with different parameters.
> +# Ensure that strace -e faultwith=<syscall>:<occurrence>:<error>,... works.
> +
> +. "${srcdir=.}/init.sh"
> +
> +INV_SC_NAME="invalid_syscall_name"
> +INV_ER_NAME="invalid_error_name"
> +INV_OCC_NBR1="-1"
> +INV_OCC_NBR2="105%"
> +ERR_MSG='invalid fault argument'
> +$(./is_x86)
> +X86=$?
> +
> +if [ ! $X86 ] # Parser check
> +then
> + skip_ "fault parsing not available for non x86 architecture"
> +else
> + if [ $X86 == 1 ] # 32 bit
> + then
> + run_strace -a0 -e trace=kill -e fault=37:2:22 -e signal=none ./fault at
> > "$EXP"
> + match_diff "$EXP" "$LOG"
> + elif [ $X86 == 2 ] # 64 bit
> + then
> + run_strace -a0 -e trace=kill -e fault=62:2:22 -e signal=none ./fault at
> > "$EXP"
> + match_diff "$EXP" "$LOG"
> + fi
> + rm $EXP $LOG
This hardcoding of syscall numbers is awful and it doesn't scale anyway.
Why can't the helper just return __NR_kill?
> + $STRACE -a0 -e trace=kill -e fault="$INV_SC_NAME":2:EINVAL ./fault at 2>
> "$LOG"
> + grep -v "$ERR_MSG" < "$LOG" && dump_log_and_fail_with 'strace-fault
> failed'
> + $STRACE -a0 -e trace=kill -e fault=kill:2:"$INV_ER_NAME" ./fault at 2>
> "$LOG"
> + grep -v "$ERR_MSG" < "$LOG" && dump_log_and_fail_with 'strace-fault
> failed'
> + $STRACE -a0 -e trace=kill -e fault=kill:"$INV_OCC_NBR1":EAGAIN ./fault
> at 2> "$LOG"
> + grep -v "$ERR_MSG" < "$LOG" && dump_log_and_fail_with 'strace-fault
> failed'
> + $STRACE -a0 -e trace=kill -e fault=kill:"$INV_OCC_NBR2":EAGAIN ./fault
> at 2> "$LOG"
> + grep -v "$ERR_MSG" < "$LOG" && dump_log_and_fail_with 'strace-fault
> failed'
A common test function would be much easier to read.
--
ldv
pgp0w5dJzZePT.pgp
Description: PGP signature
------------------------------------------------------------------------------
_______________________________________________ Strace-devel mailing list Strace-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/strace-devel
