* [LTP] [PATCH v1] recvmsg01: Refactor using new LTP API
@ 2023-03-29 13:38 Wei Gao via ltp
2023-03-30 5:17 ` Petr Vorel
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Wei Gao via ltp @ 2023-03-29 13:38 UTC (permalink / raw)
To: ltp
Signed-off-by: Wei Gao <wegao@suse.com>
---
testcases/kernel/syscalls/recvmsg/recvmsg01.c | 370 +++++++-----------
1 file changed, 143 insertions(+), 227 deletions(-)
diff --git a/testcases/kernel/syscalls/recvmsg/recvmsg01.c b/testcases/kernel/syscalls/recvmsg/recvmsg01.c
index 3ce7ab027..4da9c123a 100644
--- a/testcases/kernel/syscalls/recvmsg/recvmsg01.c
+++ b/testcases/kernel/syscalls/recvmsg/recvmsg01.c
@@ -1,51 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
/*
- *
- * Copyright (c) International Business Machines Corp., 2001
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
- * the GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ * Copyright (c) 2001 International Business Machines
+ * Copyright (c) 2023 Wei Gao <wegao@suse.com>
*/
-/*
- * Test Name: recvmsg01
+/*\
+ * [Description]
*
- * Test Description:
- * Verify that recvmsg() returns the proper errno for various failure cases
- *
- * Usage: <for command-line>
- * recvmsg01 [-c n] [-e] [-i n] [-I x] [-P x] [-t]
- * where, -c n : Run n copies concurrently.
- * -e : Turn on errno logging.
- * -i n : Execute test n times.
- * -I x : Execute test for x seconds.
- * -P x : Pause for x seconds between iterations.
- * -t : Turn on syscall timing.
+ * Verify that recvmsg() returns the proper errno for various failure cases
*
* HISTORY
* 07/2001 Ported by Wayne Boyer
- *
- * RESTRICTIONS:
- * None.
- *
*/
+
#include <stdio.h>
#include <unistd.h>
#include <string.h>
#include <errno.h>
#include <fcntl.h>
-
+#include <stdlib.h>
#include <sys/wait.h>
#include <sys/types.h>
#include <sys/socket.h>
@@ -54,42 +28,38 @@
#include <sys/un.h>
#include <sys/file.h>
-#include <netinet/in.h>
+#include "tst_test.h"
+#include "tst_net.h"
+#include "tst_safe_macros.h"
-#include "test.h"
-#include "safe_macros.h"
+#define TM "from recvmsg01 server"
-char *TCID = "recvmsg01";
-int testno;
-
-char buf[1024], cbuf[1024];
-int s; /* socket descriptor */
-int passed_fd = -1; /* rights-passing test descriptor */
-struct sockaddr_in sin1, from;
-struct sockaddr_un sun1;
-struct msghdr msgdat;
-struct cmsghdr *control = 0;
-int controllen = 0;
-struct iovec iov[1];
+static char buf[1024], cbuf[1024];
+static int s; /* socket descriptor */
+static struct sockaddr_in sin1, from;
+static struct sockaddr_un sun1;
+static struct msghdr msgdat;
+static struct cmsghdr *control;
+static int controllen;
+static struct iovec iov[1];
static int sfd; /* shared between do_child and start_server */
static int ufd; /* shared between do_child and start_server */
-void setup(void);
-void setup0(void);
-void setup1(void);
-void setup2(void);
-void setup3(void);
-void setup4(void);
-void cleanup(void);
-void cleanup0(void);
-void cleanup1(void);
-void cleanup2(void);
-void do_child(void);
-
-void sender(int);
-pid_t start_server(struct sockaddr_in *, struct sockaddr_un *);
-
-struct test_case_t { /* test case structure */
+static void setup(void);
+static void setup0(int);
+static void setup1(int);
+static void setup2(int);
+static void setup3(int);
+static void setup4(int);
+static void cleanup(void);
+static void cleanup0(void);
+static void cleanup1(void);
+static void cleanup2(void);
+static void do_child(void);
+static void sender(int);
+static pid_t start_server(struct sockaddr_in *, struct sockaddr_un *);
+
+static struct tcase { /* test case structure */
int domain; /* PF_INET, PF_UNIX, ... */
int type; /* SOCK_STREAM, SOCK_DGRAM ... */
int proto; /* protocol number (usually 0 = default) */
@@ -98,15 +68,15 @@ struct test_case_t { /* test case structure */
void *buf; /* recv data buffer */
int buflen; /* recv buffer length */
struct msghdr *msg;
- unsigned flags;
+ unsigned int flags;
struct sockaddr *from; /* from address */
int fromlen; /* from address value/result buffer length */
int retval; /* syscall return value */
int experrno; /* expected errno */
- void (*setup) (void);
- void (*cleanup) (void);
+ void (*setup)(int n);
+ void (*cleanup)(void);
char *desc;
-} tdat[] = {
+} tcases[] = {
/* 1 */
{
PF_INET, SOCK_STREAM, 0, iov, 1, buf, sizeof(buf), &msgdat, 0,
@@ -179,130 +149,106 @@ struct test_case_t { /* test case structure */
PF_UNIX, SOCK_STREAM, 0, iov, 1, (void *)buf, sizeof(buf),
&msgdat, 0, (struct sockaddr *)&from, sizeof(from),
0, 0, setup4, cleanup2, "large cmesg length"}
-,};
-
-int TST_TOTAL = sizeof(tdat) / sizeof(tdat[0]);
-
-#ifdef UCLINUX
-static char *argv0;
-#endif
+,
+};
-int main(int argc, char *argv[])
+static void run(unsigned int n)
{
- int lc;
-
- tst_parse_opts(argc, argv, NULL, NULL);
-#ifdef UCLINUX
- argv0 = argv[0];
- maybe_run_child(&do_child, "dd", &sfd, &ufd);
-#endif
-
setup();
- for (lc = 0; TEST_LOOPING(lc); ++lc) {
- tst_count = 0;
- for (testno = 0; testno < TST_TOTAL; ++testno) {
- if ((tst_kvercmp(3, 17, 0) < 0)
- && (tdat[testno].flags & MSG_ERRQUEUE)
- && (tdat[testno].type & SOCK_STREAM)) {
- tst_resm(TCONF, "skip MSG_ERRQUEUE test, "
- "it's supported from 3.17");
- continue;
- }
+ struct tcase *tc = &tcases[n];
- tdat[testno].setup();
-
- /* setup common to all tests */
- iov[0].iov_base = tdat[testno].buf;
- iov[0].iov_len = tdat[testno].buflen;
- msgdat.msg_name = tdat[testno].from;
- msgdat.msg_namelen = tdat[testno].fromlen;
- msgdat.msg_iov = tdat[testno].iov;
- msgdat.msg_iovlen = tdat[testno].iovcnt;
- msgdat.msg_control = control;
- msgdat.msg_controllen = controllen;
- msgdat.msg_flags = 0;
-
- TEST(recvmsg(s, tdat[testno].msg, tdat[testno].flags));
- if (TEST_RETURN >= 0)
- TEST_RETURN = 0; /* all nonzero equal here */
- if (TEST_RETURN != tdat[testno].retval ||
- (TEST_RETURN < 0 &&
- TEST_ERRNO != tdat[testno].experrno)) {
- tst_resm(TFAIL, "%s ; returned"
- " %ld (expected %d), errno %d (expected"
- " %d)", tdat[testno].desc,
- TEST_RETURN, tdat[testno].retval,
- TEST_ERRNO, tdat[testno].experrno);
- } else {
- tst_resm(TPASS, "%s successful",
- tdat[testno].desc);
- }
- tdat[testno].cleanup();
- }
+ if ((tst_kvercmp(3, 17, 0) < 0)
+ && (tc->flags & MSG_ERRQUEUE)
+ && (tc->type & SOCK_STREAM)) {
+ tst_res(TCONF, "skip MSG_ERRQUEUE test, "
+ "it's supported from 3.17");
}
- cleanup();
- tst_exit();
+ tc->setup(n);
+
+ /* setup common to all tests */
+ iov[0].iov_base = tc->buf;
+ iov[0].iov_len = tc->buflen;
+ msgdat.msg_name = tc->from;
+ msgdat.msg_namelen = tc->fromlen;
+ msgdat.msg_iov = tc->iov;
+ msgdat.msg_iovlen = tc->iovcnt;
+ msgdat.msg_control = control;
+ msgdat.msg_controllen = controllen;
+ msgdat.msg_flags = 0;
+
+ TEST(recvmsg(s, tc->msg, tc->flags));
+ if (TST_RET >= 0)
+ TST_RET = 0; /* all nonzero equal here */
+ if (TST_RET != tc->retval ||
+ (TST_RET < 0 &&
+ TST_ERR != tc->experrno)) {
+ tst_res(TFAIL, "%s ; returned"
+ " %ld (expected %d), errno %d (expected"
+ " %d)", tc->desc,
+ TST_RET, tc->retval,
+ TST_ERR, tc->experrno);
+ } else {
+ tst_res(TPASS, "%s successful",
+ tc->desc);
+ }
+ tc->cleanup();
+
+ cleanup();
}
pid_t pid;
char tmpsunpath[1024];
-void setup(void)
+static void setup(void)
{
int tfd;
- TEST_PAUSE;
- tst_tmpdir();
+ sun1.sun_family = AF_UNIX;
+
(void)strcpy(tmpsunpath, "udsockXXXXXX");
tfd = mkstemp(tmpsunpath);
- close(tfd);
- unlink(tmpsunpath);
- sun1.sun_family = AF_UNIX;
+ SAFE_CLOSE(tfd);
+ SAFE_UNLINK(tmpsunpath);
(void)strcpy(sun1.sun_path, tmpsunpath);
-
- signal(SIGPIPE, SIG_IGN);
-
+ SAFE_SIGNAL(SIGPIPE, SIG_IGN);
pid = start_server(&sin1, &sun1);
}
-void cleanup(void)
+static void cleanup(void)
{
if (pid > 0) {
(void)kill(pid, SIGKILL); /* kill server */
wait(NULL);
}
if (tmpsunpath[0] != '\0')
- (void)unlink(tmpsunpath);
- tst_rmdir();
-
+ (void)SAFE_UNLINK(tmpsunpath);
}
-void setup0(void)
+static void setup0(int n)
{
- if (tdat[testno].experrno == EBADF)
+ if (tcases[n].experrno == EBADF)
s = 400; /* anything not an open file */
- else if ((s = open("/dev/null", O_WRONLY)) == -1)
- tst_brkm(TBROK | TERRNO, cleanup, "open(/dev/null) failed");
+ else
+ s = SAFE_OPEN("/dev/null", O_WRONLY);
}
-void cleanup0(void)
+static void cleanup0(void)
{
s = -1;
}
-void setup1(void)
+static void setup1(int n)
{
fd_set rdfds;
struct timeval timeout;
- int n;
- s = SAFE_SOCKET(cleanup, tdat[testno].domain, tdat[testno].type,
- tdat[testno].proto);
- if (tdat[testno].type == SOCK_STREAM) {
- if (tdat[testno].domain == PF_INET) {
- SAFE_CONNECT(cleanup, s, (struct sockaddr *)&sin1,
+ s = SAFE_SOCKET(tcases[n].domain, tcases[n].type,
+ tcases[n].proto);
+ if (tcases[n].type == SOCK_STREAM) {
+ if (tcases[n].domain == PF_INET) {
+ SAFE_CONNECT(s, (struct sockaddr *)&sin1,
sizeof(sin1));
/* Wait for something to be readable, else we won't detect EFAULT on recv */
FD_ZERO(&rdfds);
@@ -311,54 +257,45 @@ void setup1(void)
timeout.tv_usec = 0;
n = select(s + 1, &rdfds, 0, 0, &timeout);
if (n != 1 || !FD_ISSET(s, &rdfds))
- tst_brkm(TBROK, cleanup,
- "client setup1 failed - no message ready in 2 sec");
- } else if (tdat[testno].domain == PF_UNIX) {
- SAFE_CONNECT(cleanup, s, (struct sockaddr *)&sun1,
+ tst_brk(TBROK, "client setup1 failed - no message ready in 2 sec");
+ } else if (tcases[n].domain == PF_UNIX) {
+ SAFE_CONNECT(s, (struct sockaddr *)&sun1,
sizeof(sun1));
}
}
}
-void setup2(void)
+static void setup2(int n)
{
- setup1();
- if (write(s, "R", 1) < 0)
- tst_brkm(TBROK | TERRNO, cleanup, "test setup failed: write:");
+ setup1(n);
+ SAFE_SEND(1, s, "R", 1, 0);
control = (struct cmsghdr *)cbuf;
controllen = control->cmsg_len = sizeof(cbuf);
}
-void setup3(void)
+static void setup3(int n)
{
- setup2();
+ setup2(n);
controllen = sizeof(struct cmsghdr) - 1;
}
-void setup4(void)
+static void setup4(int n)
{
- setup2();
+ setup2(n);
controllen = 128 * 1024;
}
-void cleanup1(void)
+static void cleanup1(void)
{
- (void)close(s);
- close(ufd);
- close(sfd);
+ SAFE_CLOSE(s);
s = -1;
}
-void cleanup2(void)
+static void cleanup2(void)
{
- close(ufd);
- close(sfd);
- (void)close(s);
+ SAFE_CLOSE(s);
s = -1;
- if (passed_fd >= 0)
- (void)close(passed_fd);
- passed_fd = -1;
control = 0;
controllen = 0;
}
@@ -373,63 +310,37 @@ pid_t start_server(struct sockaddr_in *ssin, struct sockaddr_un *ssun)
ssin->sin_addr.s_addr = INADDR_ANY;
/* set up inet socket */
- sfd = socket(PF_INET, SOCK_STREAM, 0);
- if (sfd < 0) {
- tst_brkm(TBROK | TERRNO, cleanup, "server socket failed");
- return -1;
- }
- if (bind(sfd, (struct sockaddr *)ssin, sizeof(*ssin)) < 0) {
- tst_brkm(TBROK | TERRNO, cleanup, "server bind failed");
- return -1;
- }
- if (listen(sfd, 10) < 0) {
- tst_brkm(TBROK | TERRNO, cleanup, "server listen failed");
- return -1;
- }
- SAFE_GETSOCKNAME(cleanup, sfd, (struct sockaddr *)ssin, &slen);
+ sfd = SAFE_SOCKET(PF_INET, SOCK_STREAM, 0);
+ SAFE_BIND(sfd, (struct sockaddr *)ssin, sizeof(*ssin));
+ SAFE_LISTEN(sfd, 10);
+ SAFE_GETSOCKNAME(sfd, (struct sockaddr *)ssin, &slen);
/* set up UNIX-domain socket */
- ufd = socket(PF_UNIX, SOCK_STREAM, 0);
- if (ufd < 0) {
- tst_brkm(TBROK | TERRNO, cleanup, "server UD socket failed");
- return -1;
- }
- if (bind(ufd, (struct sockaddr *)ssun, sizeof(*ssun))) {
- tst_brkm(TBROK | TERRNO, cleanup, "server UD bind failed");
- return -1;
- }
- if (listen(ufd, 10) < 0) {
- tst_brkm(TBROK | TERRNO, cleanup, "server UD listen failed");
- return -1;
- }
+ ufd = SAFE_SOCKET(PF_UNIX, SOCK_STREAM, 0);
+ SAFE_BIND(ufd, (struct sockaddr *)ssun, sizeof(*ssun));
+ SAFE_LISTEN(ufd, 10);
- switch ((pid = FORK_OR_VFORK())) {
+ switch ((pid = SAFE_FORK())) {
case 0: /* child */
-#ifdef UCLINUX
- if (self_exec(argv0, "dd", sfd, ufd) < 0)
- tst_brkm(TBROK | TERRNO, cleanup,
- "server self_exec failed");
-#else
do_child();
-#endif
break;
case -1:
- tst_brkm(TBROK | TERRNO, cleanup, "server fork failed");
+ tst_brk(TBROK | TERRNO, "server fork failed");
/* fall through */
default: /* parent */
- (void)close(sfd);
- (void)close(ufd);
+ SAFE_CLOSE(sfd);
+ SAFE_CLOSE(ufd);
return pid;
}
exit(1);
}
-void do_child(void)
+static void do_child(void)
{
struct sockaddr_in fsin;
struct sockaddr_un fsun;
fd_set afds, rfds;
- int nfds, cc, fd;
+ int nfds, fd;
FD_ZERO(&afds);
FD_SET(sfd, &afds);
@@ -455,19 +366,19 @@ void do_child(void)
int newfd;
fromlen = sizeof(fsin);
- newfd = accept(sfd, (struct sockaddr *)&fsin, &fromlen);
+ newfd = SAFE_ACCEPT(sfd, (struct sockaddr *)&fsin, &fromlen);
if (newfd >= 0) {
FD_SET(newfd, &afds);
nfds = MAX(nfds, newfd + 1);
/* send something back */
- (void)write(newfd, "hoser\n", 6);
+ SAFE_SEND(1, newfd, "hi", 2, 0);
}
}
if (FD_ISSET(ufd, &rfds)) {
int newfd;
fromlen = sizeof(fsun);
- newfd = accept(ufd, (struct sockaddr *)&fsun, &fromlen);
+ newfd = SAFE_ACCEPT(ufd, (struct sockaddr *)&fsun, &fromlen);
if (newfd >= 0) {
FD_SET(newfd, &afds);
nfds = MAX(nfds, newfd + 1);
@@ -477,21 +388,20 @@ void do_child(void)
if (fd != sfd && fd != ufd && FD_ISSET(fd, &rfds)) {
char rbuf[1024];
- cc = read(fd, rbuf, sizeof(rbuf));
- if (cc && rbuf[0] == 'R')
+ TEST(read(fd, rbuf, sizeof(rbuf)));
+ if (TST_RET > 0 && rbuf[0] == 'R')
sender(fd);
- if (cc == 0 || (cc < 0 && errno != EINTR)) {
- (void)close(fd);
+ if (TST_RET == 0 || (TST_RET < 0 && TST_ERR != EINTR)) {
+ SAFE_CLOSE(fd);
FD_CLR(fd, &afds);
}
}
+
}
}
-#define TM "from recvmsg01 server"
-
/* special for rights-passing test */
-void sender(int fd)
+static void sender(int fd)
{
struct msghdr mh;
struct cmsghdr *control;
@@ -523,7 +433,13 @@ void sender(int fd)
mh.msg_controllen = control->cmsg_len;
/* do it */
- (void)sendmsg(fd, &mh, 0);
- (void)close(tfd);
- (void)unlink(tmpfn);
+ SAFE_SENDMSG(sizeof(TM), fd, &mh, 0);
+ SAFE_CLOSE(tfd);
+ (void)SAFE_UNLINK(tmpfn);
}
+
+static struct tst_test test = { .test = run,
+ .tcnt = ARRAY_SIZE(tcases),
+ .forks_child = 1,
+ .needs_tmpdir = 1,
+};
--
2.35.3
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [LTP] [PATCH v1] recvmsg01: Refactor using new LTP API
2023-03-29 13:38 [LTP] [PATCH v1] recvmsg01: Refactor using new LTP API Wei Gao via ltp
@ 2023-03-30 5:17 ` Petr Vorel
2023-03-30 5:58 ` Petr Vorel
2023-04-24 10:30 ` [LTP] [PATCH v2] " Wei Gao via ltp
2 siblings, 0 replies; 11+ messages in thread
From: Petr Vorel @ 2023-03-30 5:17 UTC (permalink / raw)
To: Wei Gao; +Cc: ltp
Hi Wei,
Thanks for your work. A bit more cleanup will be needed.
> +++ b/testcases/kernel/syscalls/recvmsg/recvmsg01.c
> @@ -1,51 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> /*
> + * Copyright (c) 2001 International Business Machines
There should have been (we've been changing the file after it was created):
* Copyright (c) Linux Test Project, 2002-2022
> + * Copyright (c) 2023 Wei Gao <wegao@suse.com>
> */
> -/*
> - * Test Name: recvmsg01
> +/*\
> + * [Description]
> *
> - * Test Description:
> - * Verify that recvmsg() returns the proper errno for various failure cases
> - *
> - * Usage: <for command-line>
> - * recvmsg01 [-c n] [-e] [-i n] [-I x] [-P x] [-t]
> - * where, -c n : Run n copies concurrently.
> - * -e : Turn on errno logging.
> - * -i n : Execute test n times.
> - * -I x : Execute test for x seconds.
> - * -P x : Pause for x seconds between iterations.
> - * -t : Turn on syscall timing.
> + * Verify that recvmsg() returns the proper errno for various failure cases
> *
> * HISTORY
> * 07/2001 Ported by Wayne Boyer
This line should have been added to the copyright section.
Have look at the output of docparse - run:
cd metadata/ && make
and then see the output of ../docparse/metadata.html#recvmsg01
We certainly don't want this info there.
> - *
> - * RESTRICTIONS:
> - * None.
> - *
> */
> +
> #include <stdio.h>
> #include <unistd.h>
> #include <string.h>
> #include <errno.h>
> #include <fcntl.h>
> -
> +#include <stdlib.h>
> #include <sys/wait.h>
> #include <sys/types.h>
> #include <sys/socket.h>
> @@ -54,42 +28,38 @@
> #include <sys/un.h>
> #include <sys/file.h>
> -#include <netinet/in.h>
> +#include "tst_test.h"
> +#include "tst_net.h"
> +#include "tst_safe_macros.h"
> -#include "test.h"
> -#include "safe_macros.h"
Only these 4 headers are needed:
#include <stdio.h>
#include <stdlib.h>
#include <sys/wait.h>
#include "tst_test.h"
> +#define TM "from recvmsg01 server"
What is TM? Maybe MSG or other less cryptic constant name?
Also, you added struct tcase. Most of the comments are useless, could you please
remove it?
static struct tcase { /* test case structure */
int domain; /* PF_INET, PF_UNIX, ... */
int type; /* SOCK_STREAM, SOCK_DGRAM ... */
int proto; /* protocol number (usually 0 = default) */
Maybe rename this to protocol and that it's obvious without comment?
struct iovec *iov;
int iovcnt;
void *buf; /* recv data buffer */
Maybe this comment is the only relevant to keep. But name it recv_buf would
describe it without comment.
int buflen; /* recv buffer length */
struct msghdr *msg;
unsigned int flags;
struct sockaddr *from; /* from address */
int fromlen; /* from address value/result buffer length */
int retval; /* syscall return value */
Maybe just ret?
int experrno; /* expected errno */
Maybe exp_errno or just err?
void (*setup)(int n);
void (*cleanup)(void);
char *desc;
} tcases[] = {
/* 1 */
We don't comment the number of the testcase, one can find the problematic
testcase via the description.
{
PF_INET, SOCK_STREAM, 0, iov, 1, buf, sizeof(buf), &msgdat, 0,
(struct sockaddr *)&from, sizeof(from),
-1, EBADF, setup0, cleanup0, "bad file descriptor"}
,
Also, better we define structures as:
{
.domain = PF_INET,
.type = SOCK_STREAM,
.iov = iov,
.iovcnt = 1,
...
},
{
.iov = iov,
.iovcnt = 1,
...
},
...
Using struct member name is much readable.
NOTE: you can omit values which are zero.
Also, recvmsg() for "invalid socket buffer" and "invalid cmsg length" (below) passes
(return 0, errno is obviously also 0).
{
PF_INET, SOCK_STREAM, 0, iov, 1, (void *)buf, sizeof(buf),
&msgdat, 0, (struct sockaddr *)-1, sizeof(from), 0,
// TODO: ENOTSOCK, setup1, cleanup1, "invalid socket buffer"}
0, setup1, cleanup1, "invalid socket buffer"}
,
{
PF_UNIX, SOCK_STREAM, 0, iov, 1, (void *)buf, sizeof(buf),
&msgdat, 0, (struct sockaddr *)&from, sizeof(from),
// TODO: 0, EINVAL, setup3, cleanup2, "invalid cmsg length"}
0, 0, setup3, cleanup2, "invalid cmsg length"}
,
i.e. if instead of
if (TST_RET != tc->retval || (TST_RET < 0 && TST_ERR != tc->experrno)) {
we evaluate expected errno:
if (TST_RET != tc->retval || (TST_ERR != tc->experrno)) {
Either they it should be fixed (to really fail) or the testcases should be removed as invalid.
> +static char buf[1024], cbuf[1024];
Maybe replace 1024 with some constant (BUF_SIZE ?)
The code
static void setup4(int n)
{
setup2(n);
controllen = 128 * 1024;
}
> +static int s; /* socket descriptor */
Please could this be just sock?
static int sock;
Having single letter for variable which requires comment is not a good code.
> +static struct sockaddr_in sin1, from;
> +static struct sockaddr_un sun1;
> +static struct msghdr msgdat;
> +static struct cmsghdr *control;
> +static int controllen;
> +static struct iovec iov[1];
> static int sfd; /* shared between do_child and start_server */
> static int ufd; /* shared between do_child and start_server */
> +static void setup(void);
> +static void setup0(int);
Could all the specific setup and cleanup be more descriptive instead of using
number? E.g. setup_open instead of setup0 ?
> +static void setup1(int);
> +static void setup2(int);
> +static void setup3(int);
> +static void setup4(int);
> +static void cleanup(void);
> +static void cleanup0(void);
> +static void cleanup1(void);
> +static void cleanup2(void);
> +static void do_child(void);
> +static void sender(int);
I don't like defining stubs in LTP tests (stubs are for headers in libraries),
but understand that struct tcase is usually defined at the beginning, thus it's
necessary to have stubs. But at least the stub for sender() does not have to be
specified, if you move sender() function above do_child() which uses it.
> +static pid_t start_server(struct sockaddr_in *, struct sockaddr_un *);
> + tc->setup(n);
> +
> + /* setup common to all tests */
This comment is obvious, please remove it.
> + iov[0].iov_base = tc->buf;
> + iov[0].iov_len = tc->buflen;
> + msgdat.msg_name = tc->from;
> + msgdat.msg_namelen = tc->fromlen;
> + msgdat.msg_iov = tc->iov;
> + msgdat.msg_iovlen = tc->iovcnt;
> + msgdat.msg_control = control;
> + msgdat.msg_controllen = controllen;
> + msgdat.msg_flags = 0;
> +
> + TEST(recvmsg(s, tc->msg, tc->flags));
> + if (TST_RET >= 0)
> + TST_RET = 0; /* all nonzero equal here */
I'd remove the comment.
Please add space here (readability).
> + if (TST_RET != tc->retval ||
> + (TST_RET < 0 &&
> + TST_ERR != tc->experrno)) {
> + tst_res(TFAIL, "%s ; returned"
> + " %ld (expected %d), errno %d (expected"
> + " %d)", tc->desc,
> + TST_RET, tc->retval,
> + TST_ERR, tc->experrno);
I'd
* use TTERRNO for printing the actual errno (prints also constant name: EINVAL (22))
* have condition
> + } else {
> + tst_res(TPASS, "%s successful",
> + tc->desc);
> + }
> + tc->cleanup();
> +
> + cleanup();
TEST(recvmsg(s, tc->msg, tc->flags));
if (TST_RET >= 0)
TST_RET = 0;
if (TST_RET != tc->retval || (TST_RET < 0 && TST_ERR != tc->experrno)) {
tst_res(TFAIL | TTERRNO,
"%s: expected return: %d, returned: %ld, expected errno: %d",
tc->desc, tc->retval, TST_RET, tc->experrno);
} else {
tst_res(TPASS, "%s passed", tc->desc);
}
tc->cleanup();
cleanup();
> pid_t pid;
> char tmpsunpath[1024];
These two should be static.
BTW make check-recvmsg01 would tell you it.
...
> -void cleanup(void)
> +static void cleanup(void)
> {
> if (pid > 0) {
> (void)kill(pid, SIGKILL); /* kill server */
> wait(NULL);
> }
> if (tmpsunpath[0] != '\0')
> - (void)unlink(tmpsunpath);
> - tst_rmdir();
> -
> + (void)SAFE_UNLINK(tmpsunpath);
> }
> -void setup0(void)
> +static void setup0(int n)
> {
> - if (tdat[testno].experrno == EBADF)
> + if (tcases[n].experrno == EBADF)
> s = 400; /* anything not an open file */
> - else if ((s = open("/dev/null", O_WRONLY)) == -1)
> - tst_brkm(TBROK | TERRNO, cleanup, "open(/dev/null) failed");
> + else
> + s = SAFE_OPEN("/dev/null", O_WRONLY);
> }
> -void cleanup0(void)
> +static void cleanup0(void)
> {
> s = -1;
> }
> -void setup1(void)
> +static void setup1(int n)
> {
> fd_set rdfds;
> struct timeval timeout;
> - int n;
> - s = SAFE_SOCKET(cleanup, tdat[testno].domain, tdat[testno].type,
> - tdat[testno].proto);
> - if (tdat[testno].type == SOCK_STREAM) {
> - if (tdat[testno].domain == PF_INET) {
> - SAFE_CONNECT(cleanup, s, (struct sockaddr *)&sin1,
> + s = SAFE_SOCKET(tcases[n].domain, tcases[n].type,
> + tcases[n].proto);
> + if (tcases[n].type == SOCK_STREAM) {
> + if (tcases[n].domain == PF_INET) {
> + SAFE_CONNECT(s, (struct sockaddr *)&sin1,
> sizeof(sin1));
> /* Wait for something to be readable, else we won't detect EFAULT on recv */
> FD_ZERO(&rdfds);
> @@ -311,54 +257,45 @@ void setup1(void)
> timeout.tv_usec = 0;
> n = select(s + 1, &rdfds, 0, 0, &timeout);
> if (n != 1 || !FD_ISSET(s, &rdfds))
> - tst_brkm(TBROK, cleanup,
> - "client setup1 failed - no message ready in 2 sec");
> - } else if (tdat[testno].domain == PF_UNIX) {
> - SAFE_CONNECT(cleanup, s, (struct sockaddr *)&sun1,
> + tst_brk(TBROK, "client setup1 failed - no message ready in 2 sec");
> + } else if (tcases[n].domain == PF_UNIX) {
> + SAFE_CONNECT(s, (struct sockaddr *)&sun1,
> sizeof(sun1));
> }
> }
> }
> -void setup2(void)
> +static void setup2(int n)
> {
> - setup1();
> - if (write(s, "R", 1) < 0)
> - tst_brkm(TBROK | TERRNO, cleanup, "test setup failed: write:");
> + setup1(n);
> + SAFE_SEND(1, s, "R", 1, 0);
> control = (struct cmsghdr *)cbuf;
> controllen = control->cmsg_len = sizeof(cbuf);
> }
> -void setup3(void)
> +static void setup3(int n)
> {
> - setup2();
> + setup2(n);
> controllen = sizeof(struct cmsghdr) - 1;
> }
> -void setup4(void)
> +static void setup4(int n)
> {
> - setup2();
> + setup2(n);
> controllen = 128 * 1024;
> }
> -void cleanup1(void)
> +static void cleanup1(void)
> {
> - (void)close(s);
> - close(ufd);
> - close(sfd);
> + SAFE_CLOSE(s);
> s = -1;
> }
> -void cleanup2(void)
> +static void cleanup2(void)
> {
> - close(ufd);
> - close(sfd);
> - (void)close(s);
> + SAFE_CLOSE(s);
> s = -1;
> - if (passed_fd >= 0)
> - (void)close(passed_fd);
> - passed_fd = -1;
> control = 0;
> controllen = 0;
> }
> @@ -373,63 +310,37 @@ pid_t start_server(struct sockaddr_in *ssin, struct sockaddr_un *ssun)
> ssin->sin_addr.s_addr = INADDR_ANY;
> /* set up inet socket */
> - sfd = socket(PF_INET, SOCK_STREAM, 0);
> - if (sfd < 0) {
> - tst_brkm(TBROK | TERRNO, cleanup, "server socket failed");
> - return -1;
> - }
> - if (bind(sfd, (struct sockaddr *)ssin, sizeof(*ssin)) < 0) {
> - tst_brkm(TBROK | TERRNO, cleanup, "server bind failed");
> - return -1;
> - }
> - if (listen(sfd, 10) < 0) {
> - tst_brkm(TBROK | TERRNO, cleanup, "server listen failed");
> - return -1;
> - }
> - SAFE_GETSOCKNAME(cleanup, sfd, (struct sockaddr *)ssin, &slen);
> + sfd = SAFE_SOCKET(PF_INET, SOCK_STREAM, 0);
> + SAFE_BIND(sfd, (struct sockaddr *)ssin, sizeof(*ssin));
> + SAFE_LISTEN(sfd, 10);
> + SAFE_GETSOCKNAME(sfd, (struct sockaddr *)ssin, &slen);
> /* set up UNIX-domain socket */
> - ufd = socket(PF_UNIX, SOCK_STREAM, 0);
> - if (ufd < 0) {
> - tst_brkm(TBROK | TERRNO, cleanup, "server UD socket failed");
> - return -1;
> - }
> - if (bind(ufd, (struct sockaddr *)ssun, sizeof(*ssun))) {
> - tst_brkm(TBROK | TERRNO, cleanup, "server UD bind failed");
> - return -1;
> - }
> - if (listen(ufd, 10) < 0) {
> - tst_brkm(TBROK | TERRNO, cleanup, "server UD listen failed");
> - return -1;
> - }
> + ufd = SAFE_SOCKET(PF_UNIX, SOCK_STREAM, 0);
> + SAFE_BIND(ufd, (struct sockaddr *)ssun, sizeof(*ssun));
> + SAFE_LISTEN(ufd, 10);
> - switch ((pid = FORK_OR_VFORK())) {
> + switch ((pid = SAFE_FORK())) {
> case 0: /* child */
> -#ifdef UCLINUX
> - if (self_exec(argv0, "dd", sfd, ufd) < 0)
> - tst_brkm(TBROK | TERRNO, cleanup,
> - "server self_exec failed");
> -#else
> do_child();
> -#endif
> break;
> case -1:
> - tst_brkm(TBROK | TERRNO, cleanup, "server fork failed");
> + tst_brk(TBROK | TERRNO, "server fork failed");
tst_brk() for failed fork() is done in SAFE_FORK(), it is redundant here.
Therefore there is only parent and child handling, please use if for child with
exit and then parent code:
pid = SAFE_FORK();
if (!pid) {
do_child();
exit(1);
}
SAFE_CLOSE(sfd);
SAFE_CLOSE(ufd);
return pid;
> /* fall through */
> default: /* parent */
> - (void)close(sfd);
> - (void)close(ufd);
> + SAFE_CLOSE(sfd);
> + SAFE_CLOSE(ufd);
> return pid;
> }
> exit(1);
> }
> +static void do_child(void)
> {
...
> fromlen = sizeof(fsun);
> - newfd = accept(ufd, (struct sockaddr *)&fsun, &fromlen);
> + newfd = SAFE_ACCEPT(ufd, (struct sockaddr *)&fsun, &fromlen);
> if (newfd >= 0) {
> FD_SET(newfd, &afds);
> nfds = MAX(nfds, newfd + 1);
> @@ -477,21 +388,20 @@ void do_child(void)
> if (fd != sfd && fd != ufd && FD_ISSET(fd, &rfds)) {
> char rbuf[1024];
Maybe replace 1024 with some constant (BUF_SIZE).
> - cc = read(fd, rbuf, sizeof(rbuf));
> - if (cc && rbuf[0] == 'R')
> + TEST(read(fd, rbuf, sizeof(rbuf)));
> + if (TST_RET > 0 && rbuf[0] == 'R')
> sender(fd);
> - if (cc == 0 || (cc < 0 && errno != EINTR)) {
> - (void)close(fd);
> + if (TST_RET == 0 || (TST_RET < 0 && TST_ERR != EINTR)) {
> + SAFE_CLOSE(fd);
> FD_CLR(fd, &afds);
> }
> }
> +
> }
> }
...
> +static struct tst_test test = { .test = run,
Please put ".test = run," on the separate line.
> + .tcnt = ARRAY_SIZE(tcases),
> + .forks_child = 1,
> + .needs_tmpdir = 1,
> +};
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [LTP] [PATCH v1] recvmsg01: Refactor using new LTP API
2023-03-29 13:38 [LTP] [PATCH v1] recvmsg01: Refactor using new LTP API Wei Gao via ltp
2023-03-30 5:17 ` Petr Vorel
@ 2023-03-30 5:58 ` Petr Vorel
2023-04-24 10:30 ` [LTP] [PATCH v2] " Wei Gao via ltp
2 siblings, 0 replies; 11+ messages in thread
From: Petr Vorel @ 2023-03-30 5:58 UTC (permalink / raw)
To: Wei Gao; +Cc: ltp
Hi Wei,
{
PF_INET, SOCK_STREAM, 0, iov, 1, (void *)buf, sizeof(buf),
&msgdat, -1, (struct sockaddr *)&from, -1, -1,
EINVAL, setup1, cleanup1, "invalid socket length"},
.flags is -1 here, it's unsigned int, please use UINT_MAX (the same value, but
more obvious).
> - for (lc = 0; TEST_LOOPING(lc); ++lc) {
> - tst_count = 0;
> - for (testno = 0; testno < TST_TOTAL; ++testno) {
> - if ((tst_kvercmp(3, 17, 0) < 0)
> - && (tdat[testno].flags & MSG_ERRQUEUE)
> - && (tdat[testno].type & SOCK_STREAM)) {
> - tst_resm(TCONF, "skip MSG_ERRQUEUE test, "
> - "it's supported from 3.17");
> - continue;
> - }
...
> + if ((tst_kvercmp(3, 17, 0) < 0)
> + && (tc->flags & MSG_ERRQUEUE)
> + && (tc->type & SOCK_STREAM)) {
> + tst_res(TCONF, "skip MSG_ERRQUEUE test, "
> + "it's supported from 3.17");
The reason for the MSG_ERRQUEUE on SOCK_STREAM was described in
e5fd512d8 ("skip MSG_ERRQUEUE+tcp test on kernels < 3.17")
The limitation for MSG_ERRQUEUE is only on SOCK_STREAM, but because MSG_ERRQUEUE
is tested only on SOCK_STREAM, it should be enough to check just MSG_ERRQUEUE.
Also, UINT_MAX & MSG_ERRQUEUE is always true (in "invalid socket length") and I
don't think that test should be skipped.
The check could be:
if (tc->flags != UINT_MAX && tc->flags & MSG_ERRQUEUE
&& tst_kvercmp(3, 17, 0) < 0) {
tst_res(TCONF, "MSG_ERRQUEUE support for TCP has been added in 3.17");
}
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2] recvmsg01: Refactor using new LTP API
2023-03-29 13:38 [LTP] [PATCH v1] recvmsg01: Refactor using new LTP API Wei Gao via ltp
2023-03-30 5:17 ` Petr Vorel
2023-03-30 5:58 ` Petr Vorel
@ 2023-04-24 10:30 ` Wei Gao via ltp
2023-04-28 12:46 ` Petr Vorel
2023-04-30 6:04 ` [LTP] [PATCH v3] " Wei Gao via ltp
2 siblings, 2 replies; 11+ messages in thread
From: Wei Gao via ltp @ 2023-04-24 10:30 UTC (permalink / raw)
To: ltp
---
testcases/kernel/syscalls/recvmsg/recvmsg01.c | 734 +++++++++---------
1 file changed, 365 insertions(+), 369 deletions(-)
diff --git a/testcases/kernel/syscalls/recvmsg/recvmsg01.c b/testcases/kernel/syscalls/recvmsg/recvmsg01.c
index 3ce7ab027..860b7d515 100644
--- a/testcases/kernel/syscalls/recvmsg/recvmsg01.c
+++ b/testcases/kernel/syscalls/recvmsg/recvmsg01.c
@@ -1,364 +1,383 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
/*
- *
- * Copyright (c) International Business Machines Corp., 2001
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
- * the GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ * Copyright (c) 2001 Wayne Boyer International Business Machines
+ * Copyright (c) Linux Test Project, 2002-2022
+ * Copyright (c) 2023 Wei Gao <wegao@suse.com>
*/
-/*
- * Test Name: recvmsg01
- *
- * Test Description:
- * Verify that recvmsg() returns the proper errno for various failure cases
- *
- * Usage: <for command-line>
- * recvmsg01 [-c n] [-e] [-i n] [-I x] [-P x] [-t]
- * where, -c n : Run n copies concurrently.
- * -e : Turn on errno logging.
- * -i n : Execute test n times.
- * -I x : Execute test for x seconds.
- * -P x : Pause for x seconds between iterations.
- * -t : Turn on syscall timing.
+/*\
+ * [Description]
*
- * HISTORY
- * 07/2001 Ported by Wayne Boyer
- *
- * RESTRICTIONS:
- * None.
+ * Verify that recvmsg() returns the proper errno for various failure cases
*
*/
#include <stdio.h>
-#include <unistd.h>
-#include <string.h>
-#include <errno.h>
-#include <fcntl.h>
-
+#include <stdlib.h>
#include <sys/wait.h>
-#include <sys/types.h>
-#include <sys/socket.h>
-#include <sys/signal.h>
-#include <sys/uio.h>
-#include <sys/un.h>
-#include <sys/file.h>
-
-#include <netinet/in.h>
-
-#include "test.h"
-#include "safe_macros.h"
-
-char *TCID = "recvmsg01";
-int testno;
-
-char buf[1024], cbuf[1024];
-int s; /* socket descriptor */
-int passed_fd = -1; /* rights-passing test descriptor */
-struct sockaddr_in sin1, from;
-struct sockaddr_un sun1;
-struct msghdr msgdat;
-struct cmsghdr *control = 0;
-int controllen = 0;
-struct iovec iov[1];
+#include "tst_test.h"
+
+#define MSG "from recvmsg01 server"
+#define BUF_SIZE 1024
+
+static char recv_buf[BUF_SIZE], cbuf[BUF_SIZE];
+static int sock;
+static struct sockaddr_in sin1, from;
+static struct sockaddr_un sun1;
+static struct msghdr msgdat;
+static struct cmsghdr *control;
+static int controllen;
+static struct iovec iov[1];
static int sfd; /* shared between do_child and start_server */
static int ufd; /* shared between do_child and start_server */
-
-void setup(void);
-void setup0(void);
-void setup1(void);
-void setup2(void);
-void setup3(void);
-void setup4(void);
-void cleanup(void);
-void cleanup0(void);
-void cleanup1(void);
-void cleanup2(void);
-void do_child(void);
-
-void sender(int);
-pid_t start_server(struct sockaddr_in *, struct sockaddr_un *);
-
-struct test_case_t { /* test case structure */
- int domain; /* PF_INET, PF_UNIX, ... */
- int type; /* SOCK_STREAM, SOCK_DGRAM ... */
- int proto; /* protocol number (usually 0 = default) */
+static pid_t pid;
+static char tmpsunpath[BUF_SIZE];
+
+static void setup(void);
+static void setup_invalid_sock(int);
+static void setup_valid_sock(int);
+static void setup_valid_msg_control(int);
+static void setup_large_msg_control(int);
+static void cleanup(void);
+static void cleanup_reset_sock(void);
+static void cleanup_close_sock(void);
+static void cleanup_reset_all(void);
+static void do_child(void);
+static pid_t start_server(struct sockaddr_in *, struct sockaddr_un *);
+
+static struct tcase {
+ int domain;
+ int type;
+ int protocol;
struct iovec *iov;
int iovcnt;
- void *buf; /* recv data buffer */
- int buflen; /* recv buffer length */
+ void *recv_buf;
+ int buflen;
struct msghdr *msg;
- unsigned flags;
- struct sockaddr *from; /* from address */
- int fromlen; /* from address value/result buffer length */
- int retval; /* syscall return value */
- int experrno; /* expected errno */
- void (*setup) (void);
- void (*cleanup) (void);
+ unsigned int flags;
+ struct sockaddr *from;
+ int fromlen;
+ int ret;
+ int exp_errno;
+ void (*setup)(int n);
+ void (*cleanup)(void);
char *desc;
-} tdat[] = {
-/* 1 */
+} tcases[] = {
{
- PF_INET, SOCK_STREAM, 0, iov, 1, buf, sizeof(buf), &msgdat, 0,
- (struct sockaddr *)&from, sizeof(from),
- -1, EBADF, setup0, cleanup0, "bad file descriptor"}
- ,
-/* 2 */
+ .domain = PF_INET,
+ .type = SOCK_STREAM,
+ .protocol = 0,
+ .iov = iov,
+ .iovcnt = 1,
+ .recv_buf = recv_buf,
+ .buflen = sizeof(recv_buf),
+ .msg = &msgdat,
+ .flags = 0,
+ .from = (struct sockaddr *)&from,
+ .fromlen = sizeof(from),
+ .ret = -1,
+ .exp_errno = EBADF,
+ .setup = setup_invalid_sock,
+ .cleanup = cleanup_reset_sock,
+ .desc = "bad file descriptor",
+ },
{
- 0, 0, 0, iov, 1, (void *)buf, sizeof(buf), &msgdat, 0,
- (struct sockaddr *)&from, sizeof(from),
- -1, ENOTSOCK, setup0, cleanup0, "invalid socket"}
- ,
-/* 3 */
+ .iov = iov,
+ .iovcnt = 1,
+ .recv_buf = (void *)recv_buf,
+ .buflen = sizeof(recv_buf),
+ .msg = &msgdat,
+ .flags = 0,
+ .from = (struct sockaddr *)&from,
+ .fromlen = sizeof(from),
+ .ret = -1,
+ .exp_errno = ENOTSOCK,
+ .setup = setup_invalid_sock,
+ .cleanup = cleanup_reset_sock,
+ .desc = "invalid socket",
+ },
{
- PF_INET, SOCK_STREAM, 0, iov, 1, (void *)buf, sizeof(buf),
- &msgdat, 0, (struct sockaddr *)-1, sizeof(from), 0,
- ENOTSOCK, setup1, cleanup1, "invalid socket buffer"}
- ,
-/* 4 */
+ .domain = PF_INET,
+ .type = SOCK_STREAM,
+ .iov = iov,
+ .iovcnt = 1,
+ .recv_buf = (void *)recv_buf,
+ .buflen = sizeof(recv_buf),
+ .msg = &msgdat,
+ .flags = -1,
+ .from = (struct sockaddr *)&from,
+ .fromlen = -1,
+ .ret = -1,
+ .exp_errno = EINVAL,
+ .setup = setup_valid_sock,
+ .cleanup = cleanup_close_sock,
+ .desc = "invalid socket length",
+ },
{
- PF_INET, SOCK_STREAM, 0, iov, 1, (void *)buf, sizeof(buf),
- &msgdat, -1, (struct sockaddr *)&from, -1, -1,
- EINVAL, setup1, cleanup1, "invalid socket length"},
-/* 5 */
+ .domain = PF_INET,
+ .type = SOCK_STREAM,
+ .protocol = 0,
+ .iov = iov,
+ .iovcnt = 1,
+ .recv_buf = (void *)-1,
+ .buflen = sizeof(recv_buf),
+ .msg = &msgdat,
+ .flags = 0,
+ .from = (struct sockaddr *)&from,
+ .fromlen = sizeof(from),
+ .ret = -1,
+ .exp_errno = EFAULT,
+ .setup = setup_valid_sock,
+ .cleanup = cleanup_close_sock,
+ .desc = "invalid recv buffer",
+ },
{
- PF_INET, SOCK_STREAM, 0, iov, 1, (void *)-1, sizeof(buf),
- &msgdat, 0, (struct sockaddr *)&from, sizeof(from),
- -1, EFAULT, setup1, cleanup1, "invalid recv buffer"}
- ,
-/* 6 */
+ .domain = PF_INET,
+ .type = SOCK_STREAM,
+ .protocol = 0,
+ .iov = 0,
+ .iovcnt = 1,
+ .recv_buf = recv_buf,
+ .buflen = sizeof(recv_buf),
+ .msg = &msgdat,
+ .flags = 0,
+ .from = (struct sockaddr *)&from,
+ .fromlen = sizeof(from),
+ .ret = -1,
+ .exp_errno = EFAULT,
+ .setup = setup_valid_sock,
+ .cleanup = cleanup_close_sock,
+ .desc = "invalid iovec buffer",
+ },
{
- PF_INET, SOCK_STREAM, 0, 0, 1, (void *)buf, sizeof(buf),
- &msgdat, 0, (struct sockaddr *)&from, sizeof(from),
- -1, EFAULT, setup1, cleanup1, "invalid iovec buffer"}
- ,
-/* 7 */
+ .domain = PF_INET,
+ .type = SOCK_STREAM,
+ .protocol = 0,
+ .iov = iov,
+ .iovcnt = -1,
+ .recv_buf = recv_buf,
+ .buflen = sizeof(recv_buf),
+ .msg = &msgdat,
+ .flags = 0,
+ .from = (struct sockaddr *)&from,
+ .fromlen = sizeof(from),
+ .ret = -1,
+ .exp_errno = EMSGSIZE,
+ .setup = setup_valid_sock,
+ .cleanup = cleanup_close_sock,
+ .desc = "invalid iovec count",
+ },
{
- PF_INET, SOCK_STREAM, 0, iov, -1, (void *)buf, sizeof(buf),
- &msgdat, 0, (struct sockaddr *)&from, sizeof(from),
- -1, EMSGSIZE, setup1, cleanup1, "invalid iovec count"}
- ,
-/* 8 */
+ .domain = PF_INET,
+ .type = SOCK_STREAM,
+ .protocol = 0,
+ .iov = iov,
+ .iovcnt = 1,
+ .recv_buf = recv_buf,
+ .buflen = sizeof(recv_buf),
+ .msg = &msgdat,
+ .flags = 0,
+ .from = (struct sockaddr *)&from,
+ .fromlen = sizeof(from),
+ .ret = 0,
+ .exp_errno = 0,
+ .setup = setup_valid_msg_control,
+ .cleanup = cleanup_reset_all,
+ .desc = "rights reception",
+ },
{
- PF_UNIX, SOCK_STREAM, 0, iov, 1, (void *)buf, sizeof(buf),
- &msgdat, 0, (struct sockaddr *)&from, sizeof(from),
- 0, 0, setup2, cleanup2, "rights reception"}
- ,
-/* 9 */
+ .domain = PF_INET,
+ .type = SOCK_STREAM,
+ .protocol = 0,
+ .iov = iov,
+ .iovcnt = 1,
+ .recv_buf = recv_buf,
+ .buflen = sizeof(recv_buf),
+ .msg = &msgdat,
+ .flags = MSG_OOB,
+ .from = (struct sockaddr *)&from,
+ .fromlen = sizeof(from),
+ .ret = -1,
+ .exp_errno = EINVAL,
+ .setup = setup_valid_sock,
+ .cleanup = cleanup_close_sock,
+ .desc = "invalid MSG_OOB flag set",
+ },
{
- PF_INET, SOCK_STREAM, 0, iov, 1, (void *)buf, sizeof(buf),
- &msgdat, MSG_OOB, (struct sockaddr *)&from,
- sizeof(from), -1, EINVAL, setup1, cleanup1,
- "invalid MSG_OOB flag set"}
- ,
-/* 10 */
+ .domain = PF_INET,
+ .type = SOCK_STREAM,
+ .protocol = 0,
+ .iov = iov,
+ .iovcnt = 1,
+ .recv_buf = recv_buf,
+ .buflen = sizeof(recv_buf),
+ .msg = &msgdat,
+ .flags = MSG_ERRQUEUE,
+ .from = (struct sockaddr *)&from,
+ .fromlen = sizeof(from),
+ .ret = -1,
+ .exp_errno = EAGAIN,
+ .setup = setup_valid_sock,
+ .cleanup = cleanup_close_sock,
+ .desc = "invalid MSG_ERRQUEUE flag set",
+ },
{
- PF_INET, SOCK_STREAM, 0, iov, 1, (void *)buf, sizeof(buf),
- &msgdat, MSG_ERRQUEUE, (struct sockaddr *)&from,
- sizeof(from), -1, EAGAIN, setup1, cleanup1,
- "invalid MSG_ERRQUEUE flag set"}
- ,
-/* 11 */
- {
- PF_UNIX, SOCK_STREAM, 0, iov, 1, (void *)buf, sizeof(buf),
- &msgdat, 0, (struct sockaddr *)&from, sizeof(from),
- 0, EINVAL, setup3, cleanup2, "invalid cmsg length"}
- ,
-/* 12 */
- {
- PF_UNIX, SOCK_STREAM, 0, iov, 1, (void *)buf, sizeof(buf),
- &msgdat, 0, (struct sockaddr *)&from, sizeof(from),
- 0, 0, setup4, cleanup2, "large cmesg length"}
-,};
-
-int TST_TOTAL = sizeof(tdat) / sizeof(tdat[0]);
-
-#ifdef UCLINUX
-static char *argv0;
-#endif
-
-int main(int argc, char *argv[])
+ .domain = PF_INET,
+ .type = SOCK_STREAM,
+ .protocol = 0,
+ .iov = iov,
+ .iovcnt = 1,
+ .recv_buf = recv_buf,
+ .buflen = sizeof(recv_buf),
+ .msg = &msgdat,
+ .flags = 0,
+ .from = (struct sockaddr *)&from,
+ .fromlen = sizeof(from),
+ .ret = 0,
+ .exp_errno = 0,
+ .setup = setup_large_msg_control,
+ .cleanup = cleanup_reset_sock,
+ .desc = "large cmesg length",
+ },
+
+};
+
+static void run(unsigned int n)
{
- int lc;
-
- tst_parse_opts(argc, argv, NULL, NULL);
-#ifdef UCLINUX
- argv0 = argv[0];
- maybe_run_child(&do_child, "dd", &sfd, &ufd);
-#endif
-
setup();
- for (lc = 0; TEST_LOOPING(lc); ++lc) {
- tst_count = 0;
- for (testno = 0; testno < TST_TOTAL; ++testno) {
- if ((tst_kvercmp(3, 17, 0) < 0)
- && (tdat[testno].flags & MSG_ERRQUEUE)
- && (tdat[testno].type & SOCK_STREAM)) {
- tst_resm(TCONF, "skip MSG_ERRQUEUE test, "
- "it's supported from 3.17");
- continue;
- }
+ struct tcase *tc = &tcases[n];
- tdat[testno].setup();
-
- /* setup common to all tests */
- iov[0].iov_base = tdat[testno].buf;
- iov[0].iov_len = tdat[testno].buflen;
- msgdat.msg_name = tdat[testno].from;
- msgdat.msg_namelen = tdat[testno].fromlen;
- msgdat.msg_iov = tdat[testno].iov;
- msgdat.msg_iovlen = tdat[testno].iovcnt;
- msgdat.msg_control = control;
- msgdat.msg_controllen = controllen;
- msgdat.msg_flags = 0;
-
- TEST(recvmsg(s, tdat[testno].msg, tdat[testno].flags));
- if (TEST_RETURN >= 0)
- TEST_RETURN = 0; /* all nonzero equal here */
- if (TEST_RETURN != tdat[testno].retval ||
- (TEST_RETURN < 0 &&
- TEST_ERRNO != tdat[testno].experrno)) {
- tst_resm(TFAIL, "%s ; returned"
- " %ld (expected %d), errno %d (expected"
- " %d)", tdat[testno].desc,
- TEST_RETURN, tdat[testno].retval,
- TEST_ERRNO, tdat[testno].experrno);
- } else {
- tst_resm(TPASS, "%s successful",
- tdat[testno].desc);
- }
- tdat[testno].cleanup();
- }
+ if ((tst_kvercmp(3, 17, 0) < 0)
+ && (tc->flags & MSG_ERRQUEUE)
+ && (tc->type & SOCK_STREAM)) {
+ tst_res(TCONF, "skip MSG_ERRQUEUE test, "
+ "it's supported from 3.17");
}
- cleanup();
- tst_exit();
+ tc->setup(n);
+
+ iov[0].iov_base = tc->recv_buf;
+ iov[0].iov_len = tc->buflen;
+ msgdat.msg_name = tc->from;
+ msgdat.msg_namelen = tc->fromlen;
+ msgdat.msg_iov = tc->iov;
+ msgdat.msg_iovlen = tc->iovcnt;
+ msgdat.msg_control = control;
+ msgdat.msg_controllen = controllen;
+ msgdat.msg_flags = 0;
+
+ TEST(recvmsg(sock, tc->msg, tc->flags));
+ if (TST_RET >= 0)
+ TST_RET = 0;
+
+ if (TST_RET != tc->ret ||
+ (TST_ERR != tc->exp_errno)) {
+ tst_res(TFAIL | TTERRNO,
+ "%s ; returned"
+ " %ld (expected %d), errno %d (expected"
+ " %d)", tc->desc,
+ TST_RET, tc->ret,
+ TST_ERR, tc->exp_errno);
+ } else {
+ tst_res(TPASS, "%s successful",
+ tc->desc);
+ }
+ tc->cleanup();
+
+ cleanup();
}
-pid_t pid;
-char tmpsunpath[1024];
-void setup(void)
+static void setup(void)
{
int tfd;
- TEST_PAUSE;
- tst_tmpdir();
+ sun1.sun_family = AF_UNIX;
+
(void)strcpy(tmpsunpath, "udsockXXXXXX");
tfd = mkstemp(tmpsunpath);
- close(tfd);
- unlink(tmpsunpath);
- sun1.sun_family = AF_UNIX;
+ SAFE_CLOSE(tfd);
+ SAFE_UNLINK(tmpsunpath);
(void)strcpy(sun1.sun_path, tmpsunpath);
-
- signal(SIGPIPE, SIG_IGN);
-
+ SAFE_SIGNAL(SIGPIPE, SIG_IGN);
pid = start_server(&sin1, &sun1);
}
-void cleanup(void)
+static void cleanup(void)
{
if (pid > 0) {
(void)kill(pid, SIGKILL); /* kill server */
wait(NULL);
}
if (tmpsunpath[0] != '\0')
- (void)unlink(tmpsunpath);
- tst_rmdir();
-
+ (void)SAFE_UNLINK(tmpsunpath);
}
-void setup0(void)
+static void setup_invalid_sock(int n)
{
- if (tdat[testno].experrno == EBADF)
- s = 400; /* anything not an open file */
- else if ((s = open("/dev/null", O_WRONLY)) == -1)
- tst_brkm(TBROK | TERRNO, cleanup, "open(/dev/null) failed");
+ if (tcases[n].exp_errno == EBADF)
+ sock = 400; /* anything not an open file */
+ else
+ sock = SAFE_OPEN("/dev/null", O_WRONLY);
}
-void cleanup0(void)
+static void cleanup_reset_sock(void)
{
- s = -1;
+ sock = -1;
}
-void setup1(void)
+static void setup_valid_sock(int n)
{
fd_set rdfds;
struct timeval timeout;
- int n;
- s = SAFE_SOCKET(cleanup, tdat[testno].domain, tdat[testno].type,
- tdat[testno].proto);
- if (tdat[testno].type == SOCK_STREAM) {
- if (tdat[testno].domain == PF_INET) {
- SAFE_CONNECT(cleanup, s, (struct sockaddr *)&sin1,
+ sock = SAFE_SOCKET(tcases[n].domain, tcases[n].type,
+ tcases[n].protocol);
+ if (tcases[n].type == SOCK_STREAM) {
+ if (tcases[n].domain == PF_INET) {
+ SAFE_CONNECT(sock, (struct sockaddr *)&sin1,
sizeof(sin1));
/* Wait for something to be readable, else we won't detect EFAULT on recv */
FD_ZERO(&rdfds);
- FD_SET(s, &rdfds);
+ FD_SET(sock, &rdfds);
timeout.tv_sec = 2;
timeout.tv_usec = 0;
- n = select(s + 1, &rdfds, 0, 0, &timeout);
- if (n != 1 || !FD_ISSET(s, &rdfds))
- tst_brkm(TBROK, cleanup,
- "client setup1 failed - no message ready in 2 sec");
- } else if (tdat[testno].domain == PF_UNIX) {
- SAFE_CONNECT(cleanup, s, (struct sockaddr *)&sun1,
+ n = select(sock + 1, &rdfds, 0, 0, &timeout);
+ if (n != 1 || !FD_ISSET(sock, &rdfds))
+ tst_brk(TBROK, "client setup_valid_sock failed - no message ready in 2 sec");
+ } else if (tcases[n].domain == PF_UNIX) {
+ SAFE_CONNECT(sock, (struct sockaddr *)&sun1,
sizeof(sun1));
}
}
}
-void setup2(void)
+static void setup_valid_msg_control(int n)
{
- setup1();
- if (write(s, "R", 1) < 0)
- tst_brkm(TBROK | TERRNO, cleanup, "test setup failed: write:");
+ setup_valid_sock(n);
+ SAFE_SEND(1, sock, "R", 1, 0);
control = (struct cmsghdr *)cbuf;
controllen = control->cmsg_len = sizeof(cbuf);
}
-void setup3(void)
+static void setup_large_msg_control(int n)
{
- setup2();
- controllen = sizeof(struct cmsghdr) - 1;
+ setup_valid_msg_control(n);
+ controllen = 128 * BUF_SIZE;
}
-void setup4(void)
+static void cleanup_close_sock(void)
{
- setup2();
- controllen = 128 * 1024;
+ SAFE_CLOSE(sock);
+ sock = -1;
}
-void cleanup1(void)
+static void cleanup_reset_all(void)
{
- (void)close(s);
- close(ufd);
- close(sfd);
- s = -1;
-}
+ SAFE_CLOSE(sock);
+ sock = -1;
-void cleanup2(void)
-{
- close(ufd);
- close(sfd);
- (void)close(s);
- s = -1;
-
- if (passed_fd >= 0)
- (void)close(passed_fd);
- passed_fd = -1;
control = 0;
controllen = 0;
}
@@ -373,63 +392,73 @@ pid_t start_server(struct sockaddr_in *ssin, struct sockaddr_un *ssun)
ssin->sin_addr.s_addr = INADDR_ANY;
/* set up inet socket */
- sfd = socket(PF_INET, SOCK_STREAM, 0);
- if (sfd < 0) {
- tst_brkm(TBROK | TERRNO, cleanup, "server socket failed");
- return -1;
- }
- if (bind(sfd, (struct sockaddr *)ssin, sizeof(*ssin)) < 0) {
- tst_brkm(TBROK | TERRNO, cleanup, "server bind failed");
- return -1;
- }
- if (listen(sfd, 10) < 0) {
- tst_brkm(TBROK | TERRNO, cleanup, "server listen failed");
- return -1;
- }
- SAFE_GETSOCKNAME(cleanup, sfd, (struct sockaddr *)ssin, &slen);
+ sfd = SAFE_SOCKET(PF_INET, SOCK_STREAM, 0);
+ SAFE_BIND(sfd, (struct sockaddr *)ssin, sizeof(*ssin));
+ SAFE_LISTEN(sfd, 10);
+ SAFE_GETSOCKNAME(sfd, (struct sockaddr *)ssin, &slen);
/* set up UNIX-domain socket */
- ufd = socket(PF_UNIX, SOCK_STREAM, 0);
- if (ufd < 0) {
- tst_brkm(TBROK | TERRNO, cleanup, "server UD socket failed");
- return -1;
- }
- if (bind(ufd, (struct sockaddr *)ssun, sizeof(*ssun))) {
- tst_brkm(TBROK | TERRNO, cleanup, "server UD bind failed");
- return -1;
- }
- if (listen(ufd, 10) < 0) {
- tst_brkm(TBROK | TERRNO, cleanup, "server UD listen failed");
- return -1;
- }
+ ufd = SAFE_SOCKET(PF_UNIX, SOCK_STREAM, 0);
+ SAFE_BIND(ufd, (struct sockaddr *)ssun, sizeof(*ssun));
+ SAFE_LISTEN(ufd, 10);
- switch ((pid = FORK_OR_VFORK())) {
- case 0: /* child */
-#ifdef UCLINUX
- if (self_exec(argv0, "dd", sfd, ufd) < 0)
- tst_brkm(TBROK | TERRNO, cleanup,
- "server self_exec failed");
-#else
+ pid = SAFE_FORK();
+ if (!pid) {
do_child();
-#endif
- break;
- case -1:
- tst_brkm(TBROK | TERRNO, cleanup, "server fork failed");
- /* fall through */
- default: /* parent */
- (void)close(sfd);
- (void)close(ufd);
- return pid;
+ exit(1);
}
+
+ SAFE_CLOSE(sfd);
+ SAFE_CLOSE(ufd);
+
+ return pid;
exit(1);
}
-void do_child(void)
+/* special for rights-passing test */
+static void sender(int fd)
+{
+ struct msghdr mh;
+ struct cmsghdr *control;
+ char tmpfn[BUF_SIZE], snd_cbuf[BUF_SIZE];
+ int tfd;
+
+ (void)strcpy(tmpfn, "smtXXXXXX");
+ tfd = mkstemp(tmpfn);
+ if (tfd < 0)
+ return;
+
+ memset(&mh, 0x00, sizeof(mh));
+
+ /* set up cmsghdr */
+ control = (struct cmsghdr *)snd_cbuf;
+ memset(control, 0x00, sizeof(struct cmsghdr));
+ control->cmsg_len = sizeof(struct cmsghdr) + 4;
+ control->cmsg_level = SOL_SOCKET;
+ control->cmsg_type = SCM_RIGHTS;
+ *(int *)CMSG_DATA(control) = tfd;
+
+ /* set up msghdr */
+ iov[0].iov_base = MSG;
+ iov[0].iov_len = sizeof(MSG);
+ mh.msg_iov = iov;
+ mh.msg_iovlen = 1;
+ mh.msg_flags = 0;
+ mh.msg_control = control;
+ mh.msg_controllen = control->cmsg_len;
+
+ /* do it */
+ SAFE_SENDMSG(sizeof(MSG), fd, &mh, 0);
+ SAFE_CLOSE(tfd);
+ (void)SAFE_UNLINK(tmpfn);
+}
+
+static void do_child(void)
{
struct sockaddr_in fsin;
struct sockaddr_un fsun;
fd_set afds, rfds;
- int nfds, cc, fd;
+ int nfds, fd;
FD_ZERO(&afds);
FD_SET(sfd, &afds);
@@ -455,19 +484,19 @@ void do_child(void)
int newfd;
fromlen = sizeof(fsin);
- newfd = accept(sfd, (struct sockaddr *)&fsin, &fromlen);
+ newfd = SAFE_ACCEPT(sfd, (struct sockaddr *)&fsin, &fromlen);
if (newfd >= 0) {
FD_SET(newfd, &afds);
nfds = MAX(nfds, newfd + 1);
/* send something back */
- (void)write(newfd, "hoser\n", 6);
+ SAFE_SEND(1, newfd, "hi", 2, 0);
}
}
if (FD_ISSET(ufd, &rfds)) {
int newfd;
fromlen = sizeof(fsun);
- newfd = accept(ufd, (struct sockaddr *)&fsun, &fromlen);
+ newfd = SAFE_ACCEPT(ufd, (struct sockaddr *)&fsun, &fromlen);
if (newfd >= 0) {
FD_SET(newfd, &afds);
nfds = MAX(nfds, newfd + 1);
@@ -475,55 +504,22 @@ void do_child(void)
}
for (fd = 0; fd < nfds; ++fd)
if (fd != sfd && fd != ufd && FD_ISSET(fd, &rfds)) {
- char rbuf[1024];
+ char rbuf[BUF_SIZE];
- cc = read(fd, rbuf, sizeof(rbuf));
- if (cc && rbuf[0] == 'R')
+ TEST(read(fd, rbuf, sizeof(rbuf)));
+ if (TST_RET > 0 && rbuf[0] == 'R')
sender(fd);
- if (cc == 0 || (cc < 0 && errno != EINTR)) {
- (void)close(fd);
+ if (TST_RET == 0 || (TST_RET < 0 && TST_ERR != EINTR)) {
+ SAFE_CLOSE(fd);
FD_CLR(fd, &afds);
}
}
}
}
-#define TM "from recvmsg01 server"
-
-/* special for rights-passing test */
-void sender(int fd)
-{
- struct msghdr mh;
- struct cmsghdr *control;
- char tmpfn[1024], snd_cbuf[1024];
- int tfd;
-
- (void)strcpy(tmpfn, "smtXXXXXX");
- tfd = mkstemp(tmpfn);
- if (tfd < 0)
- return;
-
- memset(&mh, 0x00, sizeof(mh));
-
- /* set up cmsghdr */
- control = (struct cmsghdr *)snd_cbuf;
- memset(control, 0x00, sizeof(struct cmsghdr));
- control->cmsg_len = sizeof(struct cmsghdr) + 4;
- control->cmsg_level = SOL_SOCKET;
- control->cmsg_type = SCM_RIGHTS;
- *(int *)CMSG_DATA(control) = tfd;
-
- /* set up msghdr */
- iov[0].iov_base = TM;
- iov[0].iov_len = sizeof(TM);
- mh.msg_iov = iov;
- mh.msg_iovlen = 1;
- mh.msg_flags = 0;
- mh.msg_control = control;
- mh.msg_controllen = control->cmsg_len;
-
- /* do it */
- (void)sendmsg(fd, &mh, 0);
- (void)close(tfd);
- (void)unlink(tmpfn);
-}
+static struct tst_test test = {
+ .test = run,
+ .tcnt = ARRAY_SIZE(tcases),
+ .forks_child = 1,
+ .needs_tmpdir = 1,
+};
--
2.35.3
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [LTP] [PATCH v2] recvmsg01: Refactor using new LTP API
2023-04-24 10:30 ` [LTP] [PATCH v2] " Wei Gao via ltp
@ 2023-04-28 12:46 ` Petr Vorel
2023-04-30 7:15 ` Wei Gao via ltp
2023-04-30 6:04 ` [LTP] [PATCH v3] " Wei Gao via ltp
1 sibling, 1 reply; 11+ messages in thread
From: Petr Vorel @ 2023-04-28 12:46 UTC (permalink / raw)
To: Wei Gao; +Cc: ltp
Hi Wei,
First, for some reason, with higher number of run "bad file descriptor" will
fail, returning 0 instead of -1.
It starts to fail from 200th:
$ ./recvmsg01 -i 200
...
recvmsg01.c:283: TPASS: invalid iovec count successful
recvmsg01.c:283: TPASS: rights reception successful
recvmsg01.c:283: TPASS: invalid MSG_OOB flag set successful
recvmsg01.c:283: TPASS: invalid MSG_ERRQUEUE flag set successful
recvmsg01.c:283: TPASS: large cmesg length successful
recvmsg01.c:276: TFAIL: bad file descriptor ; returned 0 (expected -1), errno 0 (expected 9): SUCCESS (0)
Summary:
passed 1999
failed 1
broken 0
skipped 0
warnings 0
I also did some review, but I didn't have time to check properly whether subject
of testing is valid (quite complex test).
> testcases/kernel/syscalls/recvmsg/recvmsg01.c | 734 +++++++++---------
...
> + * Copyright (c) 2001 Wayne Boyer International Business Machines
nit: maybe
* Copyright (c) 2001 Wayne Boyer, International Business Machines Corp.
> + * Copyright (c) Linux Test Project, 2002-2022
> + * Copyright (c) 2023 Wei Gao <wegao@suse.com>
> */
> -/*
> - * Test Name: recvmsg01
> - *
> - * Test Description:
> - * Verify that recvmsg() returns the proper errno for various failure cases
> - *
> - * Usage: <for command-line>
> - * recvmsg01 [-c n] [-e] [-i n] [-I x] [-P x] [-t]
> - * where, -c n : Run n copies concurrently.
> - * -e : Turn on errno logging.
> - * -i n : Execute test n times.
> - * -I x : Execute test for x seconds.
> - * -P x : Pause for x seconds between iterations.
> - * -t : Turn on syscall timing.
> +/*\
> + * [Description]
> *
> - * HISTORY
> - * 07/2001 Ported by Wayne Boyer
> - *
> - * RESTRICTIONS:
> - * None.
> + * Verify that recvmsg() returns the proper errno for various failure cases
nit: missing dot at the end.
> *
nit: Please remove this blank line.
> */
> +static pid_t pid;
> +static char tmpsunpath[BUF_SIZE];
> +
> +static void setup(void);
> +static void setup_invalid_sock(int);
> +static void setup_valid_sock(int);
> +static void setup_valid_msg_control(int);
> +static void setup_large_msg_control(int);
> +static void cleanup(void);
> +static void cleanup_reset_sock(void);
> +static void cleanup_close_sock(void);
> +static void cleanup_reset_all(void);
> +static void do_child(void);
> +static pid_t start_server(struct sockaddr_in *, struct sockaddr_un *);
nit: I'd personally move struct tcase below to get rid of these function
prototypes, but feel free to ignore.
> +
> +static struct tcase {
> + int domain;
> + int type;
> + int protocol;
> struct iovec *iov;
> int iovcnt;
> - void *buf; /* recv data buffer */
> - int buflen; /* recv buffer length */
> + void *recv_buf;
> + int buflen;
> struct msghdr *msg;
> - unsigned flags;
> - struct sockaddr *from; /* from address */
> - int fromlen; /* from address value/result buffer length */
> - int retval; /* syscall return value */
> - int experrno; /* expected errno */
> - void (*setup) (void);
> - void (*cleanup) (void);
> + unsigned int flags;
> + struct sockaddr *from;
> + int fromlen;
> + int ret;
> + int exp_errno;
> + void (*setup)(int n);
> + void (*cleanup)(void);
> char *desc;
> -} tdat[] = {
> -/* 1 */
> +} tcases[] = {
> {
> - PF_INET, SOCK_STREAM, 0, iov, 1, buf, sizeof(buf), &msgdat, 0,
> - (struct sockaddr *)&from, sizeof(from),
> - -1, EBADF, setup0, cleanup0, "bad file descriptor"}
> - ,
> -/* 2 */
> + .domain = PF_INET,
> + .type = SOCK_STREAM,
> + .protocol = 0,
You can omit values with 0.
> + .iov = iov,
> + .iovcnt = 1,
> + .recv_buf = recv_buf,
> + .buflen = sizeof(recv_buf),
> + .msg = &msgdat,
> + .flags = 0,
Here as well (and on many places)
> + .from = (struct sockaddr *)&from,
> + .fromlen = sizeof(from),
> + .ret = -1,
> + .exp_errno = EBADF,
> + .setup = setup_invalid_sock,
> + .cleanup = cleanup_reset_sock,
> + .desc = "bad file descriptor",
> + },
...
> +static void run(unsigned int n)
> {
> - int lc;
> -
> - tst_parse_opts(argc, argv, NULL, NULL);
> -#ifdef UCLINUX
> - argv0 = argv[0];
> - maybe_run_child(&do_child, "dd", &sfd, &ufd);
> -#endif
> -
> setup();
nit: One would think this is LTP API setup, but it's needed for all tests
(+ the same applies to cleanup()). setup_all() would made it clearer that is
used for each test, but it not important.
> + struct tcase *tc = &tcases[n];
> - tdat[testno].setup();
> -
> - /* setup common to all tests */
> - iov[0].iov_base = tdat[testno].buf;
> - iov[0].iov_len = tdat[testno].buflen;
> - msgdat.msg_name = tdat[testno].from;
> - msgdat.msg_namelen = tdat[testno].fromlen;
> - msgdat.msg_iov = tdat[testno].iov;
> - msgdat.msg_iovlen = tdat[testno].iovcnt;
> - msgdat.msg_control = control;
> - msgdat.msg_controllen = controllen;
> - msgdat.msg_flags = 0;
> -
> - TEST(recvmsg(s, tdat[testno].msg, tdat[testno].flags));
> - if (TEST_RETURN >= 0)
> - TEST_RETURN = 0; /* all nonzero equal here */
> - if (TEST_RETURN != tdat[testno].retval ||
> - (TEST_RETURN < 0 &&
> - TEST_ERRNO != tdat[testno].experrno)) {
> - tst_resm(TFAIL, "%s ; returned"
> - " %ld (expected %d), errno %d (expected"
> - " %d)", tdat[testno].desc,
> - TEST_RETURN, tdat[testno].retval,
> - TEST_ERRNO, tdat[testno].experrno);
> - } else {
> - tst_resm(TPASS, "%s successful",
> - tdat[testno].desc);
> - }
> - tdat[testno].cleanup();
> - }
> + if ((tst_kvercmp(3, 17, 0) < 0)
> + && (tc->flags & MSG_ERRQUEUE)
> + && (tc->type & SOCK_STREAM)) {
> + tst_res(TCONF, "skip MSG_ERRQUEUE test, "
> + "it's supported from 3.17");
> }
if (tst_kvercmp(3, 17, 0) < 0
&& tc->flags & MSG_ERRQUEUE
&& tc->type & SOCK_STREAM) {
tst_res(TCONF, "MSG_ERRQUEUE requires kernel >= 3.17");
}
> - cleanup();
> - tst_exit();
> + tc->setup(n);
> +
> + iov[0].iov_base = tc->recv_buf;
> + iov[0].iov_len = tc->buflen;
> + msgdat.msg_name = tc->from;
> + msgdat.msg_namelen = tc->fromlen;
> + msgdat.msg_iov = tc->iov;
> + msgdat.msg_iovlen = tc->iovcnt;
> + msgdat.msg_control = control;
> + msgdat.msg_controllen = controllen;
> + msgdat.msg_flags = 0;
> +
> + TEST(recvmsg(sock, tc->msg, tc->flags));
> + if (TST_RET >= 0)
> + TST_RET = 0;
> +
> + if (TST_RET != tc->ret ||
> + (TST_ERR != tc->exp_errno)) {
if (TST_RET != tc->ret || TST_ERR != tc->exp_errno) {
> + tst_res(TFAIL | TTERRNO,
> + "%s ; returned"
> + " %ld (expected %d), errno %d (expected"
> + " %d)", tc->desc,
> + TST_RET, tc->ret,
> + TST_ERR, tc->exp_errno);
> + } else {
> + tst_res(TPASS, "%s successful",
> + tc->desc);
> + }
Maybe split return and errno (will be easier to quickly see which one is wrong).
TEST(recvmsg(sock, tc->msg, tc->flags));
if (TST_RET >= 0)
TST_RET = 0;
if (TST_RET != tc->ret) {
tst_res(TFAIL | TTERRNO, "%s: expected %d, returned %ld",
tc->desc, tc->ret, TST_RET);
} else if (TST_ERR != tc->exp_errno) {
tst_res(TFAIL | TTERRNO,
"%s: expected %s",
tc->desc, tst_strerrno(tc->exp_errno));
} else {
tst_res(TPASS, "%s passed", tc->desc);
}
tc->cleanup();
cleanup();
> + tc->cleanup();
> +
> + cleanup();
> }
...
> +static void setup_valid_sock(int n)
> {
> fd_set rdfds;
> struct timeval timeout;
> - int n;
> - s = SAFE_SOCKET(cleanup, tdat[testno].domain, tdat[testno].type,
> - tdat[testno].proto);
> - if (tdat[testno].type == SOCK_STREAM) {
> - if (tdat[testno].domain == PF_INET) {
> - SAFE_CONNECT(cleanup, s, (struct sockaddr *)&sin1,
> + sock = SAFE_SOCKET(tcases[n].domain, tcases[n].type,
> + tcases[n].protocol);
> + if (tcases[n].type == SOCK_STREAM) {
> + if (tcases[n].domain == PF_INET) {
> + SAFE_CONNECT(sock, (struct sockaddr *)&sin1,
> sizeof(sin1));
> /* Wait for something to be readable, else we won't detect EFAULT on recv */
> FD_ZERO(&rdfds);
> - FD_SET(s, &rdfds);
> + FD_SET(sock, &rdfds);
> timeout.tv_sec = 2;
> timeout.tv_usec = 0;
> - n = select(s + 1, &rdfds, 0, 0, &timeout);
> - if (n != 1 || !FD_ISSET(s, &rdfds))
> - tst_brkm(TBROK, cleanup,
> - "client setup1 failed - no message ready in 2 sec");
> - } else if (tdat[testno].domain == PF_UNIX) {
> - SAFE_CONNECT(cleanup, s, (struct sockaddr *)&sun1,
> + n = select(sock + 1, &rdfds, 0, 0, &timeout);
> + if (n != 1 || !FD_ISSET(sock, &rdfds))
> + tst_brk(TBROK, "client setup_valid_sock failed - no message ready in 2 sec");
make check-recvmsg01
recvmsg01.c:348: WARNING: Prefer using '"%s...", __func__' to using 'setup_valid_sock', this function's name, in a string
But I'd just put:
tst_brk(TBROK, "no message ready in 2 sec");
because tst_brk() shows affected line.
> +static void setup_large_msg_control(int n)
> {
> - setup2();
> - controllen = sizeof(struct cmsghdr) - 1;
> + setup_valid_msg_control(n);
> + controllen = 128 * BUF_SIZE;
Any idea why 128? What is this number for?
> }
> -void setup4(void)
> +static void cleanup_close_sock(void)
> {
> - setup2();
> - controllen = 128 * 1024;
> + SAFE_CLOSE(sock);
> + sock = -1;
This sock = -1; is redundant, because SAFE_CLOSE() set it.
> }
> +static void cleanup_reset_all(void)
> {
> - (void)close(s);
> - close(ufd);
> - close(sfd);
> - s = -1;
> -}
> + SAFE_CLOSE(sock);
> + sock = -1;
And here as well.
...
pid_t start_server(struct sockaddr_in *ssin, struct sockaddr_un *ssun)
{
pid_t pid;
socklen_t slen = sizeof(*ssin);
...
SAFE_CLOSE(sfd);
SAFE_CLOSE(ufd);
return pid;
exit(1);
=> IMHO this exit 1 is redundant and should be removed.
}
> -void do_child(void)
> +/* special for rights-passing test */
> +static void sender(int fd)
> +{
> + struct msghdr mh;
Set struct with {}
struct msghdr mh = {};
> + struct cmsghdr *control;
> + char tmpfn[BUF_SIZE], snd_cbuf[BUF_SIZE];
> + int tfd;
> +
> + (void)strcpy(tmpfn, "smtXXXXXX");
> + tfd = mkstemp(tmpfn);
> + if (tfd < 0)
> + return;
> +
> + memset(&mh, 0x00, sizeof(mh));
... will allow to skip memset here.
Also this memset will not be needed:
memset(control, 0x00, sizeof(struct cmsghdr));
> +static void do_child(void)
> {
> struct sockaddr_in fsin;
> struct sockaddr_un fsun;
> fd_set afds, rfds;
> - int nfds, cc, fd;
> + int nfds, fd;
> FD_ZERO(&afds);
> FD_SET(sfd, &afds);
> @@ -455,19 +484,19 @@ void do_child(void)
> int newfd;
> fromlen = sizeof(fsin);
> - newfd = accept(sfd, (struct sockaddr *)&fsin, &fromlen);
> + newfd = SAFE_ACCEPT(sfd, (struct sockaddr *)&fsin, &fromlen);
> if (newfd >= 0) {
> FD_SET(newfd, &afds);
> nfds = MAX(nfds, newfd + 1);
> /* send something back */
> - (void)write(newfd, "hoser\n", 6);
> + SAFE_SEND(1, newfd, "hi", 2, 0);
> }
> }
> if (FD_ISSET(ufd, &rfds)) {
> int newfd;
> fromlen = sizeof(fsun);
> - newfd = accept(ufd, (struct sockaddr *)&fsun, &fromlen);
> + newfd = SAFE_ACCEPT(ufd, (struct sockaddr *)&fsun, &fromlen);
> if (newfd >= 0) {
> FD_SET(newfd, &afds);
> nfds = MAX(nfds, newfd + 1);
> @@ -475,55 +504,22 @@ void do_child(void)
> }
> for (fd = 0; fd < nfds; ++fd)
> if (fd != sfd && fd != ufd && FD_ISSET(fd, &rfds)) {
> - char rbuf[1024];
> + char rbuf[BUF_SIZE];
> - cc = read(fd, rbuf, sizeof(rbuf));
> - if (cc && rbuf[0] == 'R')
> + TEST(read(fd, rbuf, sizeof(rbuf)));
> + if (TST_RET > 0 && rbuf[0] == 'R')
> sender(fd);
> - if (cc == 0 || (cc < 0 && errno != EINTR)) {
> - (void)close(fd);
> + if (TST_RET == 0 || (TST_RET < 0 && TST_ERR != EINTR)) {
> + SAFE_CLOSE(fd);
Please use simple close() here:
/* Here cannot be SAFE_CLOSE() which sets fd to -1,
* otherwise glibc complains:
* *** bit out of range 0 - FD_SETSIZE on fd_set ***: terminated
*/
close(fd);
With higher numbers of -i you get error message from:
glibc >= 2.36 [1]:
$ ./recvmsg01 -i 500
...
recvmsg01.c:283: TPASS: invalid socket length successful
*** bit out of range 0 - FD_SETSIZE on fd_set ***: terminated
recvmsg01.c:283: TPASS: invalid recv buffer successful
FYI The problem is somehow related to max FD_SETSIZE, see man 2 select [2]:
WARNING: select() can monitor only file descriptors numbers that
are less than FD_SETSIZE (1024)—an unreasonably low limit for
many modern applications—and this limitation will not change.
All modern applications should instead use poll(2) or epoll(7),
which do not suffer this limitation.
But in our case I suppose it's really setting the file descriptor to -1.
At least the warning disappears when using just close(fd).
Kind regards,
Petr
[1] https://sourceware.org/git/?p=glibc.git;a=blob;f=debug/fdelt_chk.c;h=d1d3a19460fb99212f91f3c9e5a5d17e593530ca;hb=HEAD#l26
[2] https://man7.org/linux/man-pages/man2/select.2.html
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v3] recvmsg01: Refactor using new LTP API
2023-04-24 10:30 ` [LTP] [PATCH v2] " Wei Gao via ltp
2023-04-28 12:46 ` Petr Vorel
@ 2023-04-30 6:04 ` Wei Gao via ltp
2023-05-02 10:31 ` Petr Vorel
2023-05-02 10:32 ` Petr Vorel
1 sibling, 2 replies; 11+ messages in thread
From: Wei Gao via ltp @ 2023-04-30 6:04 UTC (permalink / raw)
To: ltp
---
testcases/kernel/syscalls/recvmsg/recvmsg01.c | 717 +++++++++---------
1 file changed, 345 insertions(+), 372 deletions(-)
diff --git a/testcases/kernel/syscalls/recvmsg/recvmsg01.c b/testcases/kernel/syscalls/recvmsg/recvmsg01.c
index 3ce7ab027..e77a12400 100644
--- a/testcases/kernel/syscalls/recvmsg/recvmsg01.c
+++ b/testcases/kernel/syscalls/recvmsg/recvmsg01.c
@@ -1,364 +1,363 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
/*
- *
- * Copyright (c) International Business Machines Corp., 2001
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
- * the GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ * Copyright (c) 2001 Wayne Boyer International Business Machines
+ * Copyright (c) Linux Test Project, 2002-2022
+ * Copyright (c) 2023 Wei Gao <wegao@suse.com>
*/
-/*
- * Test Name: recvmsg01
- *
- * Test Description:
- * Verify that recvmsg() returns the proper errno for various failure cases
- *
- * Usage: <for command-line>
- * recvmsg01 [-c n] [-e] [-i n] [-I x] [-P x] [-t]
- * where, -c n : Run n copies concurrently.
- * -e : Turn on errno logging.
- * -i n : Execute test n times.
- * -I x : Execute test for x seconds.
- * -P x : Pause for x seconds between iterations.
- * -t : Turn on syscall timing.
- *
- * HISTORY
- * 07/2001 Ported by Wayne Boyer
- *
- * RESTRICTIONS:
- * None.
+/*\
+ * [Description]
*
+ * Verify that recvmsg() returns the proper errno for various failure cases.
*/
#include <stdio.h>
-#include <unistd.h>
-#include <string.h>
-#include <errno.h>
-#include <fcntl.h>
-
+#include <stdlib.h>
#include <sys/wait.h>
-#include <sys/types.h>
-#include <sys/socket.h>
-#include <sys/signal.h>
-#include <sys/uio.h>
-#include <sys/un.h>
-#include <sys/file.h>
-
-#include <netinet/in.h>
-
-#include "test.h"
-#include "safe_macros.h"
-
-char *TCID = "recvmsg01";
-int testno;
-
-char buf[1024], cbuf[1024];
-int s; /* socket descriptor */
-int passed_fd = -1; /* rights-passing test descriptor */
-struct sockaddr_in sin1, from;
-struct sockaddr_un sun1;
-struct msghdr msgdat;
-struct cmsghdr *control = 0;
-int controllen = 0;
-struct iovec iov[1];
+#include "tst_test.h"
+
+#define MSG "from recvmsg01 server"
+#define BUF_SIZE 1024
+#define CONTROL_LEN (128 * 1024)
+
+static char recv_buf[BUF_SIZE], cbuf[BUF_SIZE];
+static int sock;
+static struct sockaddr_in sin1, from;
+static struct sockaddr_un sun1;
+static struct msghdr msgdat;
+static struct cmsghdr *control;
+static int controllen;
+static struct iovec iov[1];
static int sfd; /* shared between do_child and start_server */
static int ufd; /* shared between do_child and start_server */
-
-void setup(void);
-void setup0(void);
-void setup1(void);
-void setup2(void);
-void setup3(void);
-void setup4(void);
-void cleanup(void);
-void cleanup0(void);
-void cleanup1(void);
-void cleanup2(void);
-void do_child(void);
-
-void sender(int);
-pid_t start_server(struct sockaddr_in *, struct sockaddr_un *);
-
-struct test_case_t { /* test case structure */
- int domain; /* PF_INET, PF_UNIX, ... */
- int type; /* SOCK_STREAM, SOCK_DGRAM ... */
- int proto; /* protocol number (usually 0 = default) */
+static pid_t pid;
+static char tmpsunpath[BUF_SIZE];
+
+static void setup_all(void);
+static void setup_invalid_sock(int);
+static void setup_valid_sock(int);
+static void setup_valid_msg_control(int);
+static void setup_large_msg_control(int);
+static void cleanup_all(void);
+static void cleanup_invalid_sock(int);
+static void cleanup_close_sock(int);
+static void cleanup_reset_all(int);
+static void do_child(void);
+static pid_t start_server(struct sockaddr_in *, struct sockaddr_un *);
+
+static struct tcase {
+ int domain;
+ int type;
+ int protocol;
struct iovec *iov;
int iovcnt;
- void *buf; /* recv data buffer */
- int buflen; /* recv buffer length */
+ void *recv_buf;
+ int buflen;
struct msghdr *msg;
- unsigned flags;
- struct sockaddr *from; /* from address */
- int fromlen; /* from address value/result buffer length */
- int retval; /* syscall return value */
- int experrno; /* expected errno */
- void (*setup) (void);
- void (*cleanup) (void);
+ unsigned int flags;
+ struct sockaddr *from;
+ int fromlen;
+ int ret;
+ int exp_errno;
+ void (*setup)(int n);
+ void (*cleanup)(int n);
char *desc;
-} tdat[] = {
-/* 1 */
- {
- PF_INET, SOCK_STREAM, 0, iov, 1, buf, sizeof(buf), &msgdat, 0,
- (struct sockaddr *)&from, sizeof(from),
- -1, EBADF, setup0, cleanup0, "bad file descriptor"}
- ,
-/* 2 */
+} tcases[] = {
{
- 0, 0, 0, iov, 1, (void *)buf, sizeof(buf), &msgdat, 0,
- (struct sockaddr *)&from, sizeof(from),
- -1, ENOTSOCK, setup0, cleanup0, "invalid socket"}
- ,
-/* 3 */
+ .domain = PF_INET,
+ .type = SOCK_STREAM,
+ .iov = iov,
+ .iovcnt = 1,
+ .recv_buf = recv_buf,
+ .buflen = sizeof(recv_buf),
+ .msg = &msgdat,
+ .from = (struct sockaddr *)&from,
+ .fromlen = sizeof(from),
+ .ret = -1,
+ .exp_errno = EBADF,
+ .setup = setup_invalid_sock,
+ .cleanup = cleanup_invalid_sock,
+ .desc = "bad file descriptor",
+ },
{
- PF_INET, SOCK_STREAM, 0, iov, 1, (void *)buf, sizeof(buf),
- &msgdat, 0, (struct sockaddr *)-1, sizeof(from), 0,
- ENOTSOCK, setup1, cleanup1, "invalid socket buffer"}
- ,
-/* 4 */
+ .domain = PF_INET,
+ .type = SOCK_STREAM,
+ .iov = iov,
+ .iovcnt = 1,
+ .recv_buf = (void *)recv_buf,
+ .buflen = sizeof(recv_buf),
+ .msg = &msgdat,
+ .from = (struct sockaddr *)&from,
+ .fromlen = sizeof(from),
+ .ret = -1,
+ .exp_errno = ENOTSOCK,
+ .setup = setup_invalid_sock,
+ .cleanup = cleanup_invalid_sock,
+ .desc = "invalid socket",
+ },
{
- PF_INET, SOCK_STREAM, 0, iov, 1, (void *)buf, sizeof(buf),
- &msgdat, -1, (struct sockaddr *)&from, -1, -1,
- EINVAL, setup1, cleanup1, "invalid socket length"},
-/* 5 */
+ .domain = PF_INET,
+ .type = SOCK_STREAM,
+ .iov = iov,
+ .iovcnt = 1,
+ .recv_buf = (void *)recv_buf,
+ .buflen = sizeof(recv_buf),
+ .msg = &msgdat,
+ .flags = -1,
+ .from = (struct sockaddr *)&from,
+ .fromlen = -1,
+ .ret = -1,
+ .exp_errno = EINVAL,
+ .setup = setup_valid_sock,
+ .cleanup = cleanup_close_sock,
+ .desc = "invalid socket length",
+ },
{
- PF_INET, SOCK_STREAM, 0, iov, 1, (void *)-1, sizeof(buf),
- &msgdat, 0, (struct sockaddr *)&from, sizeof(from),
- -1, EFAULT, setup1, cleanup1, "invalid recv buffer"}
- ,
-/* 6 */
+ .domain = PF_INET,
+ .type = SOCK_STREAM,
+ .iov = iov,
+ .iovcnt = 1,
+ .recv_buf = (void *)-1,
+ .buflen = sizeof(recv_buf),
+ .msg = &msgdat,
+ .from = (struct sockaddr *)&from,
+ .fromlen = sizeof(from),
+ .ret = -1,
+ .exp_errno = EFAULT,
+ .setup = setup_valid_sock,
+ .cleanup = cleanup_close_sock,
+ .desc = "invalid recv buffer",
+ },
{
- PF_INET, SOCK_STREAM, 0, 0, 1, (void *)buf, sizeof(buf),
- &msgdat, 0, (struct sockaddr *)&from, sizeof(from),
- -1, EFAULT, setup1, cleanup1, "invalid iovec buffer"}
- ,
-/* 7 */
+ .domain = PF_INET,
+ .type = SOCK_STREAM,
+ .iovcnt = 1,
+ .recv_buf = recv_buf,
+ .buflen = sizeof(recv_buf),
+ .msg = &msgdat,
+ .from = (struct sockaddr *)&from,
+ .fromlen = sizeof(from),
+ .ret = -1,
+ .exp_errno = EFAULT,
+ .setup = setup_valid_sock,
+ .cleanup = cleanup_close_sock,
+ .desc = "invalid iovec buffer",
+ },
{
- PF_INET, SOCK_STREAM, 0, iov, -1, (void *)buf, sizeof(buf),
- &msgdat, 0, (struct sockaddr *)&from, sizeof(from),
- -1, EMSGSIZE, setup1, cleanup1, "invalid iovec count"}
- ,
-/* 8 */
+ .domain = PF_INET,
+ .type = SOCK_STREAM,
+ .iov = iov,
+ .iovcnt = -1,
+ .recv_buf = recv_buf,
+ .buflen = sizeof(recv_buf),
+ .msg = &msgdat,
+ .from = (struct sockaddr *)&from,
+ .fromlen = sizeof(from),
+ .ret = -1,
+ .exp_errno = EMSGSIZE,
+ .setup = setup_valid_sock,
+ .cleanup = cleanup_close_sock,
+ .desc = "invalid iovec count",
+ },
{
- PF_UNIX, SOCK_STREAM, 0, iov, 1, (void *)buf, sizeof(buf),
- &msgdat, 0, (struct sockaddr *)&from, sizeof(from),
- 0, 0, setup2, cleanup2, "rights reception"}
- ,
-/* 9 */
+ .domain = PF_INET,
+ .type = SOCK_STREAM,
+ .iov = iov,
+ .iovcnt = 1,
+ .recv_buf = recv_buf,
+ .buflen = sizeof(recv_buf),
+ .msg = &msgdat,
+ .from = (struct sockaddr *)&from,
+ .fromlen = sizeof(from),
+ .setup = setup_valid_msg_control,
+ .cleanup = cleanup_reset_all,
+ .desc = "rights reception",
+ },
{
- PF_INET, SOCK_STREAM, 0, iov, 1, (void *)buf, sizeof(buf),
- &msgdat, MSG_OOB, (struct sockaddr *)&from,
- sizeof(from), -1, EINVAL, setup1, cleanup1,
- "invalid MSG_OOB flag set"}
- ,
-/* 10 */
+ .domain = PF_INET,
+ .type = SOCK_STREAM,
+ .iov = iov,
+ .iovcnt = 1,
+ .recv_buf = recv_buf,
+ .buflen = sizeof(recv_buf),
+ .msg = &msgdat,
+ .flags = MSG_OOB,
+ .from = (struct sockaddr *)&from,
+ .fromlen = sizeof(from),
+ .ret = -1,
+ .exp_errno = EINVAL,
+ .setup = setup_valid_sock,
+ .cleanup = cleanup_close_sock,
+ .desc = "invalid MSG_OOB flag set",
+ },
{
- PF_INET, SOCK_STREAM, 0, iov, 1, (void *)buf, sizeof(buf),
- &msgdat, MSG_ERRQUEUE, (struct sockaddr *)&from,
- sizeof(from), -1, EAGAIN, setup1, cleanup1,
- "invalid MSG_ERRQUEUE flag set"}
- ,
-/* 11 */
+ .domain = PF_INET,
+ .type = SOCK_STREAM,
+ .iov = iov,
+ .iovcnt = 1,
+ .recv_buf = recv_buf,
+ .buflen = sizeof(recv_buf),
+ .msg = &msgdat,
+ .flags = MSG_ERRQUEUE,
+ .from = (struct sockaddr *)&from,
+ .fromlen = sizeof(from),
+ .ret = -1,
+ .exp_errno = EAGAIN,
+ .setup = setup_valid_sock,
+ .cleanup = cleanup_close_sock,
+ .desc = "invalid MSG_ERRQUEUE flag set",
+ },
{
- PF_UNIX, SOCK_STREAM, 0, iov, 1, (void *)buf, sizeof(buf),
- &msgdat, 0, (struct sockaddr *)&from, sizeof(from),
- 0, EINVAL, setup3, cleanup2, "invalid cmsg length"}
- ,
-/* 12 */
- {
- PF_UNIX, SOCK_STREAM, 0, iov, 1, (void *)buf, sizeof(buf),
- &msgdat, 0, (struct sockaddr *)&from, sizeof(from),
- 0, 0, setup4, cleanup2, "large cmesg length"}
-,};
-
-int TST_TOTAL = sizeof(tdat) / sizeof(tdat[0]);
+ .domain = PF_INET,
+ .type = SOCK_STREAM,
+ .iov = iov,
+ .iovcnt = 1,
+ .recv_buf = recv_buf,
+ .buflen = sizeof(recv_buf),
+ .msg = &msgdat,
+ .from = (struct sockaddr *)&from,
+ .fromlen = sizeof(from),
+ .setup = setup_large_msg_control,
+ .cleanup = cleanup_reset_all,
+ .desc = "large cmesg length",
+ },
+
+};
+
+static void run(unsigned int n)
+{
+ setup_all();
-#ifdef UCLINUX
-static char *argv0;
-#endif
+ struct tcase *tc = &tcases[n];
-int main(int argc, char *argv[])
-{
- int lc;
-
- tst_parse_opts(argc, argv, NULL, NULL);
-#ifdef UCLINUX
- argv0 = argv[0];
- maybe_run_child(&do_child, "dd", &sfd, &ufd);
-#endif
-
- setup();
-
- for (lc = 0; TEST_LOOPING(lc); ++lc) {
- tst_count = 0;
- for (testno = 0; testno < TST_TOTAL; ++testno) {
- if ((tst_kvercmp(3, 17, 0) < 0)
- && (tdat[testno].flags & MSG_ERRQUEUE)
- && (tdat[testno].type & SOCK_STREAM)) {
- tst_resm(TCONF, "skip MSG_ERRQUEUE test, "
- "it's supported from 3.17");
- continue;
- }
+ if ((tst_kvercmp(3, 17, 0) < 0)
+ && (tc->flags & MSG_ERRQUEUE)
+ && (tc->type & SOCK_STREAM)) {
+ tst_res(TCONF, "MSG_ERRQUEUE requires kernel >= 3.17");
+ }
- tdat[testno].setup();
-
- /* setup common to all tests */
- iov[0].iov_base = tdat[testno].buf;
- iov[0].iov_len = tdat[testno].buflen;
- msgdat.msg_name = tdat[testno].from;
- msgdat.msg_namelen = tdat[testno].fromlen;
- msgdat.msg_iov = tdat[testno].iov;
- msgdat.msg_iovlen = tdat[testno].iovcnt;
- msgdat.msg_control = control;
- msgdat.msg_controllen = controllen;
- msgdat.msg_flags = 0;
-
- TEST(recvmsg(s, tdat[testno].msg, tdat[testno].flags));
- if (TEST_RETURN >= 0)
- TEST_RETURN = 0; /* all nonzero equal here */
- if (TEST_RETURN != tdat[testno].retval ||
- (TEST_RETURN < 0 &&
- TEST_ERRNO != tdat[testno].experrno)) {
- tst_resm(TFAIL, "%s ; returned"
- " %ld (expected %d), errno %d (expected"
- " %d)", tdat[testno].desc,
- TEST_RETURN, tdat[testno].retval,
- TEST_ERRNO, tdat[testno].experrno);
- } else {
- tst_resm(TPASS, "%s successful",
- tdat[testno].desc);
- }
- tdat[testno].cleanup();
- }
+ tc->setup(n);
+
+ iov[0].iov_base = tc->recv_buf;
+ iov[0].iov_len = tc->buflen;
+ msgdat.msg_name = tc->from;
+ msgdat.msg_namelen = tc->fromlen;
+ msgdat.msg_iov = tc->iov;
+ msgdat.msg_iovlen = tc->iovcnt;
+ msgdat.msg_control = control;
+ msgdat.msg_controllen = controllen;
+ msgdat.msg_flags = 0;
+
+ TEST(recvmsg(sock, tc->msg, tc->flags));
+ if (TST_RET >= 0)
+ TST_RET = 0;
+
+ if (TST_RET != tc->ret) {
+ tst_res(TFAIL | TTERRNO, "%s: expected %d, returned %ld",
+ tc->desc, tc->ret, TST_RET);
+ } else if (TST_ERR != tc->exp_errno) {
+ tst_res(TFAIL | TTERRNO,
+ "%s: expected %s",
+ tc->desc, tst_strerrno(tc->exp_errno));
+ } else {
+ tst_res(TPASS, "%s passed", tc->desc);
}
- cleanup();
- tst_exit();
+ tc->cleanup(n);
+ cleanup_all();
}
-pid_t pid;
-char tmpsunpath[1024];
-void setup(void)
+static void setup_all(void)
{
int tfd;
- TEST_PAUSE;
- tst_tmpdir();
+ sun1.sun_family = AF_UNIX;
+
(void)strcpy(tmpsunpath, "udsockXXXXXX");
tfd = mkstemp(tmpsunpath);
- close(tfd);
- unlink(tmpsunpath);
- sun1.sun_family = AF_UNIX;
+ SAFE_CLOSE(tfd);
+ SAFE_UNLINK(tmpsunpath);
(void)strcpy(sun1.sun_path, tmpsunpath);
-
- signal(SIGPIPE, SIG_IGN);
-
+ SAFE_SIGNAL(SIGPIPE, SIG_IGN);
pid = start_server(&sin1, &sun1);
}
-void cleanup(void)
+static void cleanup_all(void)
{
if (pid > 0) {
(void)kill(pid, SIGKILL); /* kill server */
wait(NULL);
}
if (tmpsunpath[0] != '\0')
- (void)unlink(tmpsunpath);
- tst_rmdir();
-
+ (void)SAFE_UNLINK(tmpsunpath);
}
-void setup0(void)
+static void setup_invalid_sock(int n)
{
- if (tdat[testno].experrno == EBADF)
- s = 400; /* anything not an open file */
- else if ((s = open("/dev/null", O_WRONLY)) == -1)
- tst_brkm(TBROK | TERRNO, cleanup, "open(/dev/null) failed");
+ if (tcases[n].exp_errno == EBADF)
+ sock = 400; /* anything not an open file */
+ else
+ sock = SAFE_OPEN("/dev/null", O_WRONLY);
}
-void cleanup0(void)
+static void cleanup_invalid_sock(int n)
{
- s = -1;
+ if (tcases[n].exp_errno == EBADF)
+ sock = -1;
+ else
+ SAFE_CLOSE(sock);
}
-void setup1(void)
+static void setup_valid_sock(int n)
{
fd_set rdfds;
struct timeval timeout;
- int n;
- s = SAFE_SOCKET(cleanup, tdat[testno].domain, tdat[testno].type,
- tdat[testno].proto);
- if (tdat[testno].type == SOCK_STREAM) {
- if (tdat[testno].domain == PF_INET) {
- SAFE_CONNECT(cleanup, s, (struct sockaddr *)&sin1,
+ sock = SAFE_SOCKET(tcases[n].domain, tcases[n].type,
+ tcases[n].protocol);
+ if (tcases[n].type == SOCK_STREAM) {
+ if (tcases[n].domain == PF_INET) {
+ SAFE_CONNECT(sock, (struct sockaddr *)&sin1,
sizeof(sin1));
/* Wait for something to be readable, else we won't detect EFAULT on recv */
FD_ZERO(&rdfds);
- FD_SET(s, &rdfds);
+ FD_SET(sock, &rdfds);
timeout.tv_sec = 2;
timeout.tv_usec = 0;
- n = select(s + 1, &rdfds, 0, 0, &timeout);
- if (n != 1 || !FD_ISSET(s, &rdfds))
- tst_brkm(TBROK, cleanup,
- "client setup1 failed - no message ready in 2 sec");
- } else if (tdat[testno].domain == PF_UNIX) {
- SAFE_CONNECT(cleanup, s, (struct sockaddr *)&sun1,
+ n = select(sock + 1, &rdfds, 0, 0, &timeout);
+ if (n != 1 || !FD_ISSET(sock, &rdfds))
+ tst_brk(TBROK, "no message ready in 2 sec");
+ } else if (tcases[n].domain == PF_UNIX) {
+ SAFE_CONNECT(sock, (struct sockaddr *)&sun1,
sizeof(sun1));
}
}
}
-void setup2(void)
+static void setup_valid_msg_control(int n)
{
- setup1();
- if (write(s, "R", 1) < 0)
- tst_brkm(TBROK | TERRNO, cleanup, "test setup failed: write:");
+ setup_valid_sock(n);
+ SAFE_SEND(1, sock, "R", 1, 0);
control = (struct cmsghdr *)cbuf;
controllen = control->cmsg_len = sizeof(cbuf);
}
-void setup3(void)
+static void setup_large_msg_control(int n)
{
- setup2();
- controllen = sizeof(struct cmsghdr) - 1;
+ setup_valid_msg_control(n);
+ controllen = CONTROL_LEN;
}
-void setup4(void)
+static void cleanup_close_sock(int n)
{
- setup2();
- controllen = 128 * 1024;
+ SAFE_CLOSE(sock);
}
-void cleanup1(void)
+static void cleanup_reset_all(int n)
{
- (void)close(s);
- close(ufd);
- close(sfd);
- s = -1;
-}
+ SAFE_CLOSE(sock);
-void cleanup2(void)
-{
- close(ufd);
- close(sfd);
- (void)close(s);
- s = -1;
-
- if (passed_fd >= 0)
- (void)close(passed_fd);
- passed_fd = -1;
control = 0;
controllen = 0;
}
@@ -373,63 +372,70 @@ pid_t start_server(struct sockaddr_in *ssin, struct sockaddr_un *ssun)
ssin->sin_addr.s_addr = INADDR_ANY;
/* set up inet socket */
- sfd = socket(PF_INET, SOCK_STREAM, 0);
- if (sfd < 0) {
- tst_brkm(TBROK | TERRNO, cleanup, "server socket failed");
- return -1;
- }
- if (bind(sfd, (struct sockaddr *)ssin, sizeof(*ssin)) < 0) {
- tst_brkm(TBROK | TERRNO, cleanup, "server bind failed");
- return -1;
- }
- if (listen(sfd, 10) < 0) {
- tst_brkm(TBROK | TERRNO, cleanup, "server listen failed");
- return -1;
- }
- SAFE_GETSOCKNAME(cleanup, sfd, (struct sockaddr *)ssin, &slen);
+ sfd = SAFE_SOCKET(PF_INET, SOCK_STREAM, 0);
+ SAFE_BIND(sfd, (struct sockaddr *)ssin, sizeof(*ssin));
+ SAFE_LISTEN(sfd, 10);
+ SAFE_GETSOCKNAME(sfd, (struct sockaddr *)ssin, &slen);
/* set up UNIX-domain socket */
- ufd = socket(PF_UNIX, SOCK_STREAM, 0);
- if (ufd < 0) {
- tst_brkm(TBROK | TERRNO, cleanup, "server UD socket failed");
- return -1;
- }
- if (bind(ufd, (struct sockaddr *)ssun, sizeof(*ssun))) {
- tst_brkm(TBROK | TERRNO, cleanup, "server UD bind failed");
- return -1;
- }
- if (listen(ufd, 10) < 0) {
- tst_brkm(TBROK | TERRNO, cleanup, "server UD listen failed");
- return -1;
- }
+ ufd = SAFE_SOCKET(PF_UNIX, SOCK_STREAM, 0);
+ SAFE_BIND(ufd, (struct sockaddr *)ssun, sizeof(*ssun));
+ SAFE_LISTEN(ufd, 10);
- switch ((pid = FORK_OR_VFORK())) {
- case 0: /* child */
-#ifdef UCLINUX
- if (self_exec(argv0, "dd", sfd, ufd) < 0)
- tst_brkm(TBROK | TERRNO, cleanup,
- "server self_exec failed");
-#else
+ pid = SAFE_FORK();
+ if (!pid) {
do_child();
-#endif
- break;
- case -1:
- tst_brkm(TBROK | TERRNO, cleanup, "server fork failed");
- /* fall through */
- default: /* parent */
- (void)close(sfd);
- (void)close(ufd);
- return pid;
+ exit(1);
}
- exit(1);
+
+ SAFE_CLOSE(sfd);
+ SAFE_CLOSE(ufd);
+
+ return pid;
}
-void do_child(void)
+/* special for rights-passing test */
+static void sender(int fd)
+{
+ struct msghdr mh = {};
+ struct cmsghdr *control;
+ char tmpfn[BUF_SIZE] = "";
+ char snd_cbuf[BUF_SIZE] = "";
+ int tfd;
+
+ (void)strcpy(tmpfn, "smtXXXXXX");
+ tfd = mkstemp(tmpfn);
+ if (tfd < 0)
+ return;
+
+ /* set up cmsghdr */
+ control = (struct cmsghdr *)snd_cbuf;
+ control->cmsg_len = sizeof(struct cmsghdr) + 4;
+ control->cmsg_level = SOL_SOCKET;
+ control->cmsg_type = SCM_RIGHTS;
+ *(int *)CMSG_DATA(control) = tfd;
+
+ /* set up msghdr */
+ iov[0].iov_base = MSG;
+ iov[0].iov_len = sizeof(MSG);
+ mh.msg_iov = iov;
+ mh.msg_iovlen = 1;
+ mh.msg_flags = 0;
+ mh.msg_control = control;
+ mh.msg_controllen = control->cmsg_len;
+
+ /* do it */
+ SAFE_SENDMSG(sizeof(MSG), fd, &mh, 0);
+ SAFE_CLOSE(tfd);
+ (void)SAFE_UNLINK(tmpfn);
+}
+
+static void do_child(void)
{
struct sockaddr_in fsin;
struct sockaddr_un fsun;
fd_set afds, rfds;
- int nfds, cc, fd;
+ int nfds, fd;
FD_ZERO(&afds);
FD_SET(sfd, &afds);
@@ -455,19 +461,19 @@ void do_child(void)
int newfd;
fromlen = sizeof(fsin);
- newfd = accept(sfd, (struct sockaddr *)&fsin, &fromlen);
+ newfd = SAFE_ACCEPT(sfd, (struct sockaddr *)&fsin, &fromlen);
if (newfd >= 0) {
FD_SET(newfd, &afds);
nfds = MAX(nfds, newfd + 1);
/* send something back */
- (void)write(newfd, "hoser\n", 6);
+ SAFE_SEND(1, newfd, "hi", 2, 0);
}
}
if (FD_ISSET(ufd, &rfds)) {
int newfd;
fromlen = sizeof(fsun);
- newfd = accept(ufd, (struct sockaddr *)&fsun, &fromlen);
+ newfd = SAFE_ACCEPT(ufd, (struct sockaddr *)&fsun, &fromlen);
if (newfd >= 0) {
FD_SET(newfd, &afds);
nfds = MAX(nfds, newfd + 1);
@@ -475,55 +481,22 @@ void do_child(void)
}
for (fd = 0; fd < nfds; ++fd)
if (fd != sfd && fd != ufd && FD_ISSET(fd, &rfds)) {
- char rbuf[1024];
+ char rbuf[BUF_SIZE];
- cc = read(fd, rbuf, sizeof(rbuf));
- if (cc && rbuf[0] == 'R')
+ TEST(read(fd, rbuf, sizeof(rbuf)));
+ if (TST_RET > 0 && rbuf[0] == 'R')
sender(fd);
- if (cc == 0 || (cc < 0 && errno != EINTR)) {
- (void)close(fd);
+ if (TST_RET == 0 || (TST_RET < 0 && TST_ERR != EINTR)) {
+ close(fd);
FD_CLR(fd, &afds);
}
}
}
}
-#define TM "from recvmsg01 server"
-
-/* special for rights-passing test */
-void sender(int fd)
-{
- struct msghdr mh;
- struct cmsghdr *control;
- char tmpfn[1024], snd_cbuf[1024];
- int tfd;
-
- (void)strcpy(tmpfn, "smtXXXXXX");
- tfd = mkstemp(tmpfn);
- if (tfd < 0)
- return;
-
- memset(&mh, 0x00, sizeof(mh));
-
- /* set up cmsghdr */
- control = (struct cmsghdr *)snd_cbuf;
- memset(control, 0x00, sizeof(struct cmsghdr));
- control->cmsg_len = sizeof(struct cmsghdr) + 4;
- control->cmsg_level = SOL_SOCKET;
- control->cmsg_type = SCM_RIGHTS;
- *(int *)CMSG_DATA(control) = tfd;
-
- /* set up msghdr */
- iov[0].iov_base = TM;
- iov[0].iov_len = sizeof(TM);
- mh.msg_iov = iov;
- mh.msg_iovlen = 1;
- mh.msg_flags = 0;
- mh.msg_control = control;
- mh.msg_controllen = control->cmsg_len;
-
- /* do it */
- (void)sendmsg(fd, &mh, 0);
- (void)close(tfd);
- (void)unlink(tmpfn);
-}
+static struct tst_test test = {
+ .test = run,
+ .tcnt = ARRAY_SIZE(tcases),
+ .forks_child = 1,
+ .needs_tmpdir = 1,
+};
--
2.35.3
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [LTP] [PATCH v2] recvmsg01: Refactor using new LTP API
2023-04-28 12:46 ` Petr Vorel
@ 2023-04-30 7:15 ` Wei Gao via ltp
0 siblings, 0 replies; 11+ messages in thread
From: Wei Gao via ltp @ 2023-04-30 7:15 UTC (permalink / raw)
To: Petr Vorel; +Cc: ltp
On Fri, Apr 28, 2023 at 02:46:48PM +0200, Petr Vorel wrote:
> Hi Wei,
>
> First, for some reason, with higher number of run "bad file descriptor" will
> fail, returning 0 instead of -1.
> It starts to fail from 200th:
> $ ./recvmsg01 -i 200
>
> ...
> recvmsg01.c:283: TPASS: invalid iovec count successful
> recvmsg01.c:283: TPASS: rights reception successful
> recvmsg01.c:283: TPASS: invalid MSG_OOB flag set successful
> recvmsg01.c:283: TPASS: invalid MSG_ERRQUEUE flag set successful
> recvmsg01.c:283: TPASS: large cmesg length successful
> recvmsg01.c:276: TFAIL: bad file descriptor ; returned 0 (expected -1), errno 0 (expected 9): SUCCESS (0)
>
> Summary:
> passed 1999
> failed 1
> broken 0
> skipped 0
> warnings 0
>
> I also did some review, but I didn't have time to check properly whether subject
> of testing is valid (quite complex test).
>
Thanks for your careful test.
I spent lot of time to investigat why this happen, finally i found it caused by wrong
cleanup of the sock fd.
I also put this fix in v3 patch.
> > +static void setup_large_msg_control(int n)
> > {
> > - setup2();
> > - controllen = sizeof(struct cmsghdr) - 1;
> > + setup_valid_msg_control(n);
> > + controllen = 128 * BUF_SIZE;
> Any idea why 128? What is this number for?
> > }
I think this is random select number when this case first introduced.
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [LTP] [PATCH v3] recvmsg01: Refactor using new LTP API
2023-04-30 6:04 ` [LTP] [PATCH v3] " Wei Gao via ltp
@ 2023-05-02 10:31 ` Petr Vorel
2023-05-02 10:32 ` Petr Vorel
1 sibling, 0 replies; 11+ messages in thread
From: Petr Vorel @ 2023-05-02 10:31 UTC (permalink / raw)
To: Wei Gao; +Cc: ltp
Hi Wei,
code has 2 warnings:
recvmsg01.c: In function ‘cleanup_close_sock’:
recvmsg01.c:352:36: warning: unused parameter ‘n’ [-Wunused-parameter]
352 | static void cleanup_close_sock(int n)
| ~~~~^
recvmsg01.c: In function ‘cleanup_reset_all’:
recvmsg01.c:357:35: warning: unused parameter ‘n’ [-Wunused-parameter]
357 | static void cleanup_reset_all(int n)
| ~~~~^
Using LTP_ATTRIBUTE_UNUSED fixes that (it can be fixed before merge).
Please, next time, pay attention to new warnings (the goal is to *not* introduce
any more warnings).
/* special for rights-passing test */
...
{
.domain = PF_INET,
.type = SOCK_STREAM,
.iov = iov,
.iovcnt = 1,
.recv_buf = recv_buf,
.buflen = sizeof(recv_buf),
.msg = &msgdat,
.from = (struct sockaddr *)&from,
.fromlen = sizeof(from),
.setup = setup_valid_msg_control,
.cleanup = cleanup_reset_all,
.desc = "rights reception",
},
=> IMHO "permission" more formal thus better word than "rights".
+ .domain = PF_INET,
+ .type = SOCK_STREAM,
+ .iov = iov,
+ .iovcnt = -1,
+ .recv_buf = recv_buf,
+ .buflen = sizeof(recv_buf),
+ .msg = &msgdat,
+ .from = (struct sockaddr *)&from,
+ .fromlen = sizeof(from),
+ .ret = -1,
Expected return is mostly -1 (only two tests does not set it).
It could be calculated from .exp_errno in run(unsigned int n)
(it's better to avoid unnecessary members in the test struct => readability).
int ret = tc->exp_errno ? -1 : 0;
...
if (TST_RET != ret) {
tst_res(TFAIL | TTERRNO, "%s: expected %d, returned %ld",
tc->desc, ret, TST_RET);
} else if (TST_ERR != tc->exp_errno) {
...
+ .exp_errno = EMSGSIZE,
+ .setup = setup_valid_sock,
+ .cleanup = cleanup_close_sock,
+ .desc = "invalid iovec count",
+ },
I haven't seen any other issue, thus I'll wait little longer for the feedback
and then I'm going to merge with these changes (things I mentioned above and few
style / space fixes). Thanks!
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Kind regards,
Petr
diff --git testcases/kernel/syscalls/recvmsg/recvmsg01.c testcases/kernel/syscalls/recvmsg/recvmsg01.c
index e77a12400..2576e1572 100644
--- testcases/kernel/syscalls/recvmsg/recvmsg01.c
+++ testcases/kernel/syscalls/recvmsg/recvmsg01.c
@@ -57,7 +57,6 @@ static struct tcase {
unsigned int flags;
struct sockaddr *from;
int fromlen;
- int ret;
int exp_errno;
void (*setup)(int n);
void (*cleanup)(int n);
@@ -73,7 +72,6 @@ static struct tcase {
.msg = &msgdat,
.from = (struct sockaddr *)&from,
.fromlen = sizeof(from),
- .ret = -1,
.exp_errno = EBADF,
.setup = setup_invalid_sock,
.cleanup = cleanup_invalid_sock,
@@ -89,7 +87,6 @@ static struct tcase {
.msg = &msgdat,
.from = (struct sockaddr *)&from,
.fromlen = sizeof(from),
- .ret = -1,
.exp_errno = ENOTSOCK,
.setup = setup_invalid_sock,
.cleanup = cleanup_invalid_sock,
@@ -106,7 +103,6 @@ static struct tcase {
.flags = -1,
.from = (struct sockaddr *)&from,
.fromlen = -1,
- .ret = -1,
.exp_errno = EINVAL,
.setup = setup_valid_sock,
.cleanup = cleanup_close_sock,
@@ -122,7 +118,6 @@ static struct tcase {
.msg = &msgdat,
.from = (struct sockaddr *)&from,
.fromlen = sizeof(from),
- .ret = -1,
.exp_errno = EFAULT,
.setup = setup_valid_sock,
.cleanup = cleanup_close_sock,
@@ -137,7 +132,6 @@ static struct tcase {
.msg = &msgdat,
.from = (struct sockaddr *)&from,
.fromlen = sizeof(from),
- .ret = -1,
.exp_errno = EFAULT,
.setup = setup_valid_sock,
.cleanup = cleanup_close_sock,
@@ -153,7 +147,6 @@ static struct tcase {
.msg = &msgdat,
.from = (struct sockaddr *)&from,
.fromlen = sizeof(from),
- .ret = -1,
.exp_errno = EMSGSIZE,
.setup = setup_valid_sock,
.cleanup = cleanup_close_sock,
@@ -171,7 +164,7 @@ static struct tcase {
.fromlen = sizeof(from),
.setup = setup_valid_msg_control,
.cleanup = cleanup_reset_all,
- .desc = "rights reception",
+ .desc = "permission reception",
},
{
.domain = PF_INET,
@@ -184,7 +177,6 @@ static struct tcase {
.flags = MSG_OOB,
.from = (struct sockaddr *)&from,
.fromlen = sizeof(from),
- .ret = -1,
.exp_errno = EINVAL,
.setup = setup_valid_sock,
.cleanup = cleanup_close_sock,
@@ -201,7 +193,6 @@ static struct tcase {
.flags = MSG_ERRQUEUE,
.from = (struct sockaddr *)&from,
.fromlen = sizeof(from),
- .ret = -1,
.exp_errno = EAGAIN,
.setup = setup_valid_sock,
.cleanup = cleanup_close_sock,
@@ -226,9 +217,10 @@ static struct tcase {
static void run(unsigned int n)
{
- setup_all();
-
struct tcase *tc = &tcases[n];
+ int ret = tc->exp_errno ? -1 : 0;
+
+ setup_all();
if ((tst_kvercmp(3, 17, 0) < 0)
&& (tc->flags & MSG_ERRQUEUE)
@@ -252,9 +244,9 @@ static void run(unsigned int n)
if (TST_RET >= 0)
TST_RET = 0;
- if (TST_RET != tc->ret) {
+ if (TST_RET != ret) {
tst_res(TFAIL | TTERRNO, "%s: expected %d, returned %ld",
- tc->desc, tc->ret, TST_RET);
+ tc->desc, ret, TST_RET);
} else if (TST_ERR != tc->exp_errno) {
tst_res(TFAIL | TTERRNO,
"%s: expected %s",
@@ -289,6 +281,7 @@ static void cleanup_all(void)
(void)kill(pid, SIGKILL); /* kill server */
wait(NULL);
}
+
if (tmpsunpath[0] != '\0')
(void)SAFE_UNLINK(tmpsunpath);
}
@@ -314,23 +307,23 @@ static void setup_valid_sock(int n)
fd_set rdfds;
struct timeval timeout;
- sock = SAFE_SOCKET(tcases[n].domain, tcases[n].type,
- tcases[n].protocol);
+ sock = SAFE_SOCKET(tcases[n].domain, tcases[n].type, tcases[n].protocol);
+
if (tcases[n].type == SOCK_STREAM) {
if (tcases[n].domain == PF_INET) {
- SAFE_CONNECT(sock, (struct sockaddr *)&sin1,
- sizeof(sin1));
+ SAFE_CONNECT(sock, (struct sockaddr *)&sin1, sizeof(sin1));
/* Wait for something to be readable, else we won't detect EFAULT on recv */
FD_ZERO(&rdfds);
FD_SET(sock, &rdfds);
timeout.tv_sec = 2;
timeout.tv_usec = 0;
n = select(sock + 1, &rdfds, 0, 0, &timeout);
+
if (n != 1 || !FD_ISSET(sock, &rdfds))
- tst_brk(TBROK, "no message ready in 2 sec");
+ tst_brk(TBROK, "no message ready in %d sec", (int)timeout.tv_sec);
+
} else if (tcases[n].domain == PF_UNIX) {
- SAFE_CONNECT(sock, (struct sockaddr *)&sun1,
- sizeof(sun1));
+ SAFE_CONNECT(sock, (struct sockaddr *)&sun1, sizeof(sun1));
}
}
}
@@ -349,12 +342,12 @@ static void setup_large_msg_control(int n)
controllen = CONTROL_LEN;
}
-static void cleanup_close_sock(int n)
+static void cleanup_close_sock(int n LTP_ATTRIBUTE_UNUSED)
{
SAFE_CLOSE(sock);
}
-static void cleanup_reset_all(int n)
+static void cleanup_reset_all(int n LTP_ATTRIBUTE_UNUSED)
{
SAFE_CLOSE(sock);
@@ -394,7 +387,7 @@ pid_t start_server(struct sockaddr_in *ssin, struct sockaddr_un *ssun)
return pid;
}
-/* special for rights-passing test */
+/* for permission test */
static void sender(int fd)
{
struct msghdr mh = {};
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [LTP] [PATCH v3] recvmsg01: Refactor using new LTP API
2023-04-30 6:04 ` [LTP] [PATCH v3] " Wei Gao via ltp
2023-05-02 10:31 ` Petr Vorel
@ 2023-05-02 10:32 ` Petr Vorel
2023-05-02 11:53 ` Wei Gao via ltp
1 sibling, 1 reply; 11+ messages in thread
From: Petr Vorel @ 2023-05-02 10:32 UTC (permalink / raw)
To: Wei Gao; +Cc: ltp
Hi Wei,
Signed-off-by: Wei Gao <wegao@suse.com>
you started to omit your Signed-off-by: tag.
I'll add it for you, but please try to remember it next time.
Kind regards,
Petr
> ---
> testcases/kernel/syscalls/recvmsg/recvmsg01.c | 717 +++++++++---------
> 1 file changed, 345 insertions(+), 372 deletions(-)
> diff --git a/testcases/kernel/syscalls/recvmsg/recvmsg01.c b/testcases/kernel/syscalls/recvmsg/recvmsg01.c
...
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [LTP] [PATCH v3] recvmsg01: Refactor using new LTP API
2023-05-02 10:32 ` Petr Vorel
@ 2023-05-02 11:53 ` Wei Gao via ltp
2023-05-02 14:12 ` Petr Vorel
0 siblings, 1 reply; 11+ messages in thread
From: Wei Gao via ltp @ 2023-05-02 11:53 UTC (permalink / raw)
To: Petr Vorel; +Cc: ltp
On Tue, May 02, 2023 at 12:32:48PM +0200, Petr Vorel wrote:
> Hi Wei,
>
> Signed-off-by: Wei Gao <wegao@suse.com>
> you started to omit your Signed-off-by: tag.
> I'll add it for you, but please try to remember it next time.
Sorry for my careless. Will double check next time, thanks a lot!
>
> Kind regards,
> Petr
>
> > ---
> > testcases/kernel/syscalls/recvmsg/recvmsg01.c | 717 +++++++++---------
> > 1 file changed, 345 insertions(+), 372 deletions(-)
>
> > diff --git a/testcases/kernel/syscalls/recvmsg/recvmsg01.c b/testcases/kernel/syscalls/recvmsg/recvmsg01.c
> ...
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [LTP] [PATCH v3] recvmsg01: Refactor using new LTP API
2023-05-02 11:53 ` Wei Gao via ltp
@ 2023-05-02 14:12 ` Petr Vorel
0 siblings, 0 replies; 11+ messages in thread
From: Petr Vorel @ 2023-05-02 14:12 UTC (permalink / raw)
To: Wei Gao; +Cc: ltp
Hi Wei,
thanks a lot, merged with fixes I proposed!
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-05-02 14:12 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-29 13:38 [LTP] [PATCH v1] recvmsg01: Refactor using new LTP API Wei Gao via ltp
2023-03-30 5:17 ` Petr Vorel
2023-03-30 5:58 ` Petr Vorel
2023-04-24 10:30 ` [LTP] [PATCH v2] " Wei Gao via ltp
2023-04-28 12:46 ` Petr Vorel
2023-04-30 7:15 ` Wei Gao via ltp
2023-04-30 6:04 ` [LTP] [PATCH v3] " Wei Gao via ltp
2023-05-02 10:31 ` Petr Vorel
2023-05-02 10:32 ` Petr Vorel
2023-05-02 11:53 ` Wei Gao via ltp
2023-05-02 14:12 ` Petr Vorel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).