Re: [LTP] [PATCH v3] recvmsg01: Refactor using new LTP API

[Petr Vorel <pvorel@suse.cz>]

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