[PATCHv3 2/2] Add a test case for "-e write=n -e read=m" option for sendmmsg/recvmsg system calls
Dmitry V. Levin
ldv at altlinux.org
Sat Nov 8 01:03:20 UTC 2014
On Fri, Nov 07, 2014 at 08:16:03PM +0900, Masatake YAMATO wrote:
[...]
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -4,6 +4,7 @@ AM_CFLAGS = $(WARN_CFLAGS)
>
> check_PROGRAMS = \
> inet-accept-connect-send-recv \
> + mmsg \
> net-accept-connect \
> netlink_inet_diag \
> scm_rights \
> @@ -27,6 +28,7 @@ TESTS = \
> sigaction.test \
> stat.test \
> statfs.test \
> + mmsg.test \
> net.test \
> net-fd.test \
> net-yy.test \
New data file (mmsg.expected) has to be added to EXTRA_DIST.
> diff --git a/tests/mmsg.c b/tests/mmsg.c
> new file mode 100644
> index 0000000..3fb1323
> --- /dev/null
> +++ b/tests/mmsg.c
> @@ -0,0 +1,64 @@
> +#ifdef HAVE_CONFIG_H
> +# include "config.h"
> +#endif
> +#include <sys/socket.h>
> +#include <unistd.h>
> +#include <assert.h>
> +
> +int
> +main(void)
> +{
> +#if defined(HAVE_SENDMMSG)
As we check for HAVE_STRUCT_MMSGHDR in the main code, we should check it
in tests, too.
> + const int W = 0, R = 1;
Everybody expects R = 0, W = 1.
Yes, it would work either way, but writing to stdin descriptor and reading
from stdout descriptor looks weird, as well as "strace -e read=1 -e write=0".
> + int sv[2];
> + char one[] = "one";
> + char two[] = "two";
> + char three[] = "three";
> +
> + struct iovec iov[3] = {
The size specifier for iovec and mmsghdr arrays could be safely omitted
the same way as it is usually omitted for char arrays.
> + {
> + .iov_base = one,
> + .iov_len = sizeof(one) - 1
> + }, {
> + .iov_base = two,
> + .iov_len = sizeof(two) - 1
> + }, {
> + .iov_base = three,
> + .iov_len = sizeof(three) - 1
> + }
> + };
> +
> +#define n_mmh (sizeof (mmh)/sizeof (mmh[0]))
> + struct mmsghdr mmh[2] = {
Use before definition? Yes, it works for macros, but wouldn't it be
better to place n_mmh definition after definition of the mmsghdr array?
> + {
> + .msg_hdr = {
> + .msg_iov = iov + 0,
> + .msg_iovlen = 2,
> + }
> +
> + }, {
> + .msg_hdr = {
> + .msg_iov = iov + 2,
> + .msg_iovlen = 1,
> +
> + }
> + }
> +
> + };
> +
> + assert (socketpair (AF_UNIX, SOCK_DGRAM, 0, sv) == 0);
To be on the safe side, let's ensure that all standard descriptors are
already in use before this socketpair call (there is a code for this
purpose in tests/scm_rights.c).
> +
> + assert (dup2 (sv[W], W) == W);
> + assert (close (sv[W]) == 0);
> + assert (sendmmsg (W, mmh, n_mmh, 0) == n_mmh);
> + assert (close (W) == 0);
> +
> + assert (dup2 (sv[R], R) == R);
> + assert (close (sv[R]) == 0);
> + assert (recvmmsg(R, mmh, n_mmh, 0, NULL) == n_mmh);
> + assert (close (R) == 0);
We do not put space after the function in function calls.
Strictly speaking, neither "sizeof" nor "assert" are function calls,
but they follow the same rule.
The code would look a bit more clear if all dup2/close calls on sv[R] and
sv[W] descriptors would be placed before all calls on R and W descriptors.
> + return 0;
> +#else
> + return 77;
> +#endif
> +}
> diff --git a/tests/mmsg.expected b/tests/mmsg.expected
> new file mode 100644
> index 0000000..7fe433c
> --- /dev/null
> +++ b/tests/mmsg.expected
> @@ -0,0 +1,19 @@
> +sendmmsg(0, {{{msg_name(0)=NULL, msg_iov(2)=[{"one", 3}, {"two", 3}], msg_controllen=0, msg_flags=0}, 6}, {{msg_name(0)=NULL, msg_iov(1)=[{"three", 5}], msg_controllen=0, msg_flags=0}, 5}}, 2, 0) = 2
> + = 2 buffers in vector 0
> + * 3 bytes in buffer 0
> + | 00000 6f 6e 65 one |
> + * 3 bytes in buffer 1
> + | 00000 74 77 6f two |
> + = 1 buffers in vector 1
> + * 5 bytes in buffer 0
> + | 00000 74 68 72 65 65 three |
> +recvmmsg(1, {{{msg_name(0)=NULL, msg_iov(2)=[{"one", 3}, {"two", 3}], msg_controllen=0, msg_flags=0}, 6}, {{msg_name(0)=NULL, msg_iov(1)=[{"three", 5}], msg_controllen=0, msg_flags=0}, 5}}, 2, 0, NULL) = 2 (left NULL)
> + = 2 buffers in vector 0
> + * 3 bytes in buffer 0
> + | 00000 6f 6e 65 one |
> + * 3 bytes in buffer 1
> + | 00000 74 77 6f two |
> + = 1 buffers in vector 1
> + * 5 bytes in buffer 0
> + | 00000 74 68 72 65 65 three |
> ++++ exited with 0 +++
> diff --git a/tests/mmsg.test b/tests/mmsg.test
> new file mode 100755
> index 0000000..417bb53
> --- /dev/null
> +++ b/tests/mmsg.test
> @@ -0,0 +1,40 @@
> +#!/bin/sh
> +
> +# Check how iovecs in struct mmsghdr are decoded.
> +
> +. "${srcdir=.}/init.sh"
> +
> +mmsg_expected=mmsg.expected
$srcdir has to be prepended (like in case of init.sh) to support
out-of-tree builds.
> +
> +check_prog diff
> +
> +./mmsg || {
> + if [ $? -eq 77 ]; then
> + framework_skip_ 'sendmmsg/recvmmsg syscalls are not available'
> + else
> + fail_ 'mmsg failed'
> + fi
There is a trailing whitespace character here.
Please enable your local pre-commit hook (.git/hooks/pre-commit)
that would check these things automatically for you.
> +}
> +
> +[ -f ${mmsg_expected} ] || {
> + fail_ "cannot find expected output file: ${mmsg_expected}"
> +}
> +
> +[ -r ${mmsg_expected} ] || {
> + fail_ "cannot read expected output file: ${mmsg_expected}"
> +}
I'd write it using a single check:
cat "$mmsg_expected" > /dev/null ||
fail_ "$mmsg_expected is not available"
No need to check for cat, we can assume that it's available.
> +
> +rm -f $LOG.*
This test doesn't use strace -ff, so this is not needed.
> +
> +args="-e trace=recvmmsg,sendmmsg -e read=1 -e write=0 -o $LOG ./mmsg"
> +$STRACE $args || {
> + cat $LOG;
This trailing semicolon is not needed.
> + fail_ "$STRACE $args failed"
> +}
> +
> +
> +diff $LOG ${mmsg_expected} || {
> + fail_ "$STRACE $args failed to decode mmsghdr properly"
> +}
Please swap arguments: expected should be first.
--
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20141108/e7db9660/attachment.bin>
More information about the Strace-devel
mailing list