Skip to contentSkip to conversation

Rewrite msgctl testcases

RFC PATCH New Cyril Hrubis 3 participants 25 messages Jun 12, 2018 – Jun 25, 2018 Library code

lore
Share & tools
lore
#0 lore raw

cover Rewrite msgctl testcases

Cyril Hrubis (9):
tst_safe_sysv_ipc: Make safe_shmctl() and safe_msgctl() safer
tst_safe_sysv_ipc: SAFE_{SHMCTL,MSGCTL} return retval syscalls/ipc: Rewrite msgctl01 + merge msgctl06 syscalls/ipc: Rewrite msgctl02 to new library. syscalls/ipc: Convert to newlib and merge msgctl{13,03} syscalls/ipc: Rewrite + merge msgctl{04, 05} syscalls/ipc: Rewrite msgctl12 to new library syscalls/ipc: Remove msgctl07
syscalls/ipc: Fix Makefile

 include/tst_safe_sysv_ipc.h                     |  12 +-
 lib/tst_safe_sysv_ipc.c                         |  33 ++-
 runtest/ltplite                                 |   3 -
 runtest/stress.part3                            |   3 -
 runtest/syscalls                                |   4 -
 runtest/syscalls-ipc                            |   4 -
 testcases/kernel/syscalls/ipc/msgctl/.gitignore |   4 -
 testcases/kernel/syscalls/ipc/msgctl/Makefile   |   8 +-
 testcases/kernel/syscalls/ipc/msgctl/msgctl01.c | 242 ++++++++++----------
 testcases/kernel/syscalls/ipc/msgctl/msgctl02.c | 207 +++++------------
 testcases/kernel/syscalls/ipc/msgctl/msgctl03.c | 159 +++----------
 testcases/kernel/syscalls/ipc/msgctl/msgctl04.c | 249 +++++++-------------
 testcases/kernel/syscalls/ipc/msgctl/msgctl05.c | 186 ---------------
 testcases/kernel/syscalls/ipc/msgctl/msgctl06.c | 142 ------------
 testcases/kernel/syscalls/ipc/msgctl/msgctl07.c | 292 ------------------------
 testcases/kernel/syscalls/ipc/msgctl/msgctl12.c |  84 +++----
 testcases/kernel/syscalls/ipc/msgctl/msgctl13.c |  86 -------
 17 files changed, 360 insertions(+), 1358 deletions(-)
 delete mode 100644 testcases/kernel/syscalls/ipc/msgctl/msgctl05.c
 delete mode 100644 testcases/kernel/syscalls/ipc/msgctl/msgctl06.c
 delete mode 100644 testcases/kernel/syscalls/ipc/msgctl/msgctl07.c
 delete mode 100644 testcases/kernel/syscalls/ipc/msgctl/msgctl13.c
Signature · 1 line
2.13.6
#1 lore raw

RFC PATCH 1/9 tst_safe_sysv_ipc: Make safe_shmctl() and safe_msgctl() safer

We recently had a regression in Sys V IPC when IPC_STAT returned non-zero integer on success wrongly, which wasn't caught by LTP test but breaks software in the wild.

This commit changes safe_shmctl() and safe_msgctl() to check the return value with != 0 for IPC_STAT, IPC_SET and IPC_RMID and with == -1 otherwise.

---
 lib/tst_safe_sysv_ipc.c | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)
lib/tst_safe_sysv_ipc.c +27 -6
index ff53a2420..86f2d934b 100644
@@ -23,6 +23,24 @@
 #include "tst_test.h"
 #include "tst_safe_sysv_ipc.h"
 
+/*
+ * The IPC_STAT, IPC_SET and IPC_RMID can return either 0 or -1.
+ *
+ * Linux specific cmds either returns -1 on failure or positive integer
+ * either index into an kernel array or shared primitive indentifier.
+ */
+static int ret_check(int cmd, int ret)
+{
+	switch (cmd) {
+	case IPC_STAT:
+	case IPC_SET:
+	case IPC_RMID:
+		return ret != 0;
+	default:
+		return ret == -1;
+	}
+}
+
 int safe_msgget(const char *file, const int lineno, key_t key, int msgflg)
 {
 	int rval;
@@ -72,11 +90,13 @@ int safe_msgctl(const char *file, const int lineno, int msqid, int cmd,
 	int rval;
 
 	rval = msgctl(msqid, cmd, buf);
-	if (rval == -1) {
-		tst_brk(TBROK | TERRNO, "%s:%d: msgctl(%i, %i, %p) failed",
-			file, lineno, msqid, cmd, buf);
+	if (ret_check(cmd, rval)) {
+		tst_brk(TBROK | TERRNO,
+			"%s:%d: msgctl(%i, %i, %p) = %i failed",
+			file, lineno, msqid, cmd, buf, rval);
 	}
 
+
 	return rval;
 }
 
@@ -127,9 +147,10 @@ int safe_shmctl(const char *file, const int lineno, int shmid, int cmd,
 	int rval;
 
 	rval = shmctl(shmid, cmd, buf);
-	if (rval == -1) {
-		tst_brk(TBROK | TERRNO, "%s:%d: shmctl(%i, %i, %p) failed",
-			file, lineno, shmid, cmd, buf);
+	if (ret_check(cmd, rval)) {
+		tst_brk(TBROK | TERRNO,
+			"%s:%d: shmctl(%i, %i, %p) = %i failed",
+			file, lineno, shmid, cmd, buf, rval);
 	}
 
 	return rval;
Signature · 1 line
2.13.6
#2 lore raw

RFC PATCH 1/9 tst_safe_sysv_ipc: Make safe_shmctl() and safe_msgctl() safer

Hi Cyril,

Quoted 7 lines

We recently had a regression in Sys V IPC when IPC_STAT returned non-zero integer on success wrongly, which wasn't caught by LTP test but breaks software in the wild.

This commit changes safe_shmctl() and safe_msgctl() to check the return value with != 0 for IPC_STAT, IPC_SET and IPC_RMID and with == -1 otherwise.

LGTM. I'm surprised this is the first time, I'd expect it for more syscalls.

Quoted 1 line
Quoted 1 line

---

Kind regards,
Petr

#3 lore raw

RFC PATCH 2/9 tst_safe_sysv_ipc: SAFE_{SHMCTL, MSGCTL} return retval

In reply to Cyril Hrubis

These calls do return index into kernel internal array or IPC id for Linux specific cmds, hence we need to propagate the return value.

---
 include/tst_safe_sysv_ipc.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
include/tst_safe_sysv_ipc.h +6 -6
index e01957f75..4c7500531 100644
@@ -39,10 +39,10 @@ ssize_t safe_msgrcv(const char *file, const int lineno, int msqid, void *msgp,
 
 int safe_msgctl(const char *file, const int lineno, int msqid, int cmd,
 		struct msqid_ds *buf);
-#define SAFE_MSGCTL(msqid, cmd, buf) do { \
-	safe_msgctl(__FILE__, __LINE__, (msqid), (cmd), (buf)); \
+#define SAFE_MSGCTL(msqid, cmd, buf) ({ \
+	int tst_ret_ = safe_msgctl(__FILE__, __LINE__, (msqid), (cmd), (buf)); \
 	(msqid) = ((cmd) == IPC_RMID ? -1 : (msqid)); \
-	} while (0)
+	tst_ret_;})
 
 int safe_shmget(const char *file, const int lineno, key_t key, size_t size,
 		int shmflg);
@@ -59,9 +59,9 @@ int safe_shmdt(const char *file, const int lineno, const void *shmaddr);
 
 int safe_shmctl(const char *file, const int lineno, int shmid, int cmd,
 		struct shmid_ds *buf);
-#define SAFE_SHMCTL(shmid, cmd, buf) do { \
-	safe_shmctl(__FILE__, __LINE__, (shmid), (cmd), (buf)); \
+#define SAFE_SHMCTL(shmid, cmd, buf) ({ \
+	int tst_ret_ = safe_shmctl(__FILE__, __LINE__, (shmid), (cmd), (buf)); \
 	(shmid) = ((cmd) == IPC_RMID ? -1 : (shmid)); \
-	} while (0)
+	tst_ret_;})
 
 #endif /* TST_SAFE_SYSV_IPC_H__ */
Signature · 1 line
2.13.6
#5 lore raw

RFC PATCH 3/9 syscalls/ipc: Rewrite msgctl01 + merge msgctl06

In reply to Cyril Hrubis

Rewrite msgctl01 to be much more strict, now we assert all fields of the buffer we get from IPC_STAT.

This also removes msgctl06 that was testing the very same IPC_STAT command but checked different subset of the msqid_ds buffer.

---
 runtest/ltplite                                 |   1 -
 runtest/stress.part3                            |   1 -
 runtest/syscalls                                |   1 -
 runtest/syscalls-ipc                            |   1 -
 testcases/kernel/syscalls/ipc/msgctl/.gitignore |   1 -
 testcases/kernel/syscalls/ipc/msgctl/msgctl01.c | 242 ++++++++++++------------
 testcases/kernel/syscalls/ipc/msgctl/msgctl06.c | 142 --------------
 7 files changed, 122 insertions(+), 267 deletions(-)
 delete mode 100644 testcases/kernel/syscalls/ipc/msgctl/msgctl06.c
runtest/ltplite +0 -1
index 249262674..cb9517883 100644
@@ -484,7 +484,6 @@ msgctl02 msgctl02
 msgctl03 msgctl03
 msgctl04 msgctl04
 msgctl05 msgctl05
-msgctl06 msgctl06
 msgctl07 msgctl07
 
 msgget01 msgget01
runtest/stress.part3 +0 -1
index d9287197b..e97b08456 100644
@@ -400,7 +400,6 @@ msgctl02 msgctl02
 msgctl03 msgctl03
 msgctl04 msgctl04
 msgctl05 msgctl05
-msgctl06 msgctl06
 msgctl07 msgctl07
 msgstress01 msgstress01
 msgstress02 msgstress02
runtest/syscalls +0 -1
index 65c96edab..7befd5c66 100644
@@ -690,7 +690,6 @@ msgctl02 msgctl02
 msgctl03 msgctl03
 msgctl04 msgctl04
 msgctl05 msgctl05
-msgctl06 msgctl06
 msgctl07 msgctl07
 msgstress01 msgstress01
 msgstress02 msgstress02
runtest/syscalls-ipc +0 -1
index c734e239d..9ae91bb4c 100644
@@ -3,7 +3,6 @@ msgctl02 msgctl02
 msgctl03 msgctl03
 msgctl04 msgctl04
 msgctl05 msgctl05
-msgctl06 msgctl06
 msgctl07 msgctl07
 msgstress01 msgstress01
 msgstress02 msgstress02
testcases/kernel/syscalls/ipc/msgctl/.gitignore +0 -1
index 2edde5de4..257a2231b 100644
@@ -3,7 +3,6 @@
 /msgctl03
 /msgctl04
 /msgctl05
-/msgctl06
 /msgctl07
 /msgctl12
 /msgctl13
testcases/kernel/syscalls/ipc/msgctl/msgctl01.c +122 -120

275 lines not on this page · raw message

testcases/kernel/syscalls/ipc/msgctl/msgctl06.c deleted +0 -142

142 lines not on this page · raw message

Signature · 1 line
2.13.6
#6 lore raw

RFC PATCH 3/9 syscalls/ipc: Rewrite msgctl01 + merge msgctl06

Quoted 33 lines · Cyril Hrubis

​[...]

testcases/kernel/syscalls/ipc/msgctl/msgctl01.c +2 -3
-int main(int ac, char **av)
-{
-       int lc;
+       if (TEST_RETURN != 0) {
+               tst_res(TFAIL | TTERRNO, "msgctl() returned %li",

TEST_RETURN);

testcases/kernel/syscalls/ipc/msgctl/msgctl01.c +7 -2
+               return;
+       }
 
-       tst_parse_opts(ac, av, NULL, NULL);
+       tst_res(TPASS, "msgctl(IPC_STAT)");
 
-       setup();                /* global setup */
+       if (buf.msg_stime == 0)
+               tst_res(TPASS, "msg_stime = 0");
+       else
+               tst_res(TFAIL, "msg_stime = %lu", (unsigned

long)buf.msg_stime);

testcases/kernel/syscalls/ipc/msgctl/msgctl01.c +4 -1
-       /* The following loop checks looping state if -i option given */
+       if (buf.msg_rtime == 0)
+               tst_res(TPASS, "msg_rtime = 0");
+       else
+               tst_res(TFAIL, "msg_rtime = %lu", (unsigned

long)buf.msg_rtime);

testcases/kernel/syscalls/ipc/msgctl/msgctl01.c +1 -3
-       for (lc = 0; TEST_LOOPING(lc); lc++) {
-               /* reset tst_count in case we are looping */
-               tst_count = 0;
+       if (buf.msg_ctime == creat_time || buf.msg_ctime == creat_time +
Quoted 1 line

1) {

​
​I'm thinking that whether 1 second is enough for system shaking. ​If this program is running on an overload system, this maybe delay more than 1 second and test fails, is that a test defect?​

Maybe gives more flexible as:

    if (buf.msg_ctime <= creat_time && buf.msg_ctime >= creat_time - 3)

testcases/kernel/syscalls/ipc/msgctl/msgctl01.c line 66

Quoted 6 lines
(quoted diff) +2 -0
+               tst_res(TPASS, "msg_ctime = %lu, expected %lu",
+                       (unsigned long)buf.msg_ctime, (unsigned

long)creat_time);

testcases/kernel/syscalls/ipc/msgctl/msgctl01.c +2 -0
+       } else {
+               tst_res(TPASS, "msg_ctime = %lu, expected %lu",

​seems typo here? TFAIL

Quoted 6 lines

+ (unsigned long)buf.msg_ctime, (unsigned
long)creat_time);
+ }

​Beside that, I got this follow errors occasionally:

​# ./msgctl01
tst_test.c:1015: INFO: Timeout per run is 0h 05m 00s tst_safe_sysv_ipc.c:51: BROK: msgctl01.c:137: msgget(1627794347, 7b0)
failed: EEXIST​

Signature · 5 lines
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20180613/0f181e3f/attachment.html>
#7 lore raw

RFC PATCH 3/9 syscalls/ipc: Rewrite msgctl01 + merge msgctl06

Hi!

Quoted 4 lines

???
???I'm thinking that whether 1 second is enough for system shaking. ???If this program is running on an overload system, this maybe delay more than 1 second and test fails, is that a test defect????

Since we are calling just two subsequent syscalls here it's very unlikely that the difference in times will be greater than 1s even on loaded system, but for sure we can increase that to two or three that wouldn't do any harm.

Quoted 4 lines

Maybe gives more flexible as:

    if (buf.msg_ctime <= creat_time && buf.msg_ctime >= creat_time - 3)
Quoted 8 lines

testcases/kernel/syscalls/ipc/msgctl/msgctl01.c line 66

Quoted 6 lines
(quoted diff) +2 -0
+               tst_res(TPASS, "msg_ctime = %lu, expected %lu",
+                       (unsigned long)buf.msg_ctime, (unsigned

long)creat_time);

testcases/kernel/syscalls/ipc/msgctl/msgctl01.c +2 -0
+       } else {
+               tst_res(TPASS, "msg_ctime = %lu, expected %lu",

???seems typo here? TFAIL

Right, thanks for pointing it out!

Quoted 6 lines
Quoted 6 lines

+ (unsigned long)buf.msg_ctime, (unsigned
long)creat_time);
+ }

Quoted 6 lines

???Beside that, I got this follow errors occasionally:

???# ./msgctl01
tst_test.c:1015: INFO: Timeout per run is 0h 05m 00s tst_safe_sysv_ipc.c:51: BROK: msgctl01.c:137: msgget(1627794347, 7b0)
failed: EEXIST???

That means that there is some leftover message queue on the system, there may be a problem in how the IDs are allocated in the IPC test library.

Do you have any steps to reproduce the issue?

Signature · 2 lines
Cyril Hrubis
[email protected]
#8 lore raw

RFC PATCH 3/9 syscalls/ipc: Rewrite msgctl01 + merge msgctl06

Hi Cyril,

Quoted 10 lines · Cyril Hrubis

Hi!

Quoted 4 lines

???
???I'm thinking that whether 1 second is enough for system shaking. ???If this program is running on an overload system, this maybe delay more than 1 second and test fails, is that a test defect????

Since we are calling just two subsequent syscalls here it's very unlikely that the difference in times will be greater than 1s even on loaded system, but for sure we can increase that to two or three that wouldn't do any harm.

Ok, fine.

Quoted 29 lines
Quoted 5 lines

Maybe gives more flexible as:

    if (buf.msg_ctime <= creat_time && buf.msg_ctime >= creat_time - 3)
Quoted 8 lines

testcases/kernel/syscalls/ipc/msgctl/msgctl01.c line 66

Quoted 6 lines
(quoted diff) +2 -0
+               tst_res(TPASS, "msg_ctime = %lu, expected %lu",
+                       (unsigned long)buf.msg_ctime, (unsigned

long)creat_time);

testcases/kernel/syscalls/ipc/msgctl/msgctl01.c +2 -0
+       } else {
+               tst_res(TPASS, "msg_ctime = %lu, expected %lu",

???seems typo here? TFAIL

Right, thanks for pointing it out!

Quoted 6 lines
Quoted 6 lines

+ (unsigned long)buf.msg_ctime, (unsigned
long)creat_time);
+ }

Quoted 6 lines

???Beside that, I got this follow errors occasionally:

???# ./msgctl01
tst_test.c:1015: INFO: Timeout per run is 0h 05m 00s tst_safe_sysv_ipc.c:51: BROK: msgctl01.c:137: msgget(1627794347, 7b0)
failed: EEXIST???

Quoted 5 lines

That means that there is some leftover message queue on the system, there may be a problem in how the IDs are allocated in the IPC test library.

Do you have any steps to reproduce the issue?

No, but I will look it closely then.

BTW, this failure is not caused by your patch. I'm sure that I can reproduce it without your patch apply at that moment.

And, for the patch set, LGTM.

Signature · 2 lines
Regards,
Li Wang
#9 lore raw

RFC PATCH 3/9 syscalls/ipc: Rewrite msgctl01 + merge msgctl06

In reply to Cyril Hrubis

Hi Cyril,

Quoted 7 lines

Rewrite msgctl01 to be much more strict, now we assert all fields of the buffer we get from IPC_STAT.

This also removes msgctl06 that was testing the very same IPC_STAT command but checked different subset of the msqid_ds buffer.

Quoted 1 line

---

Nice improvement :).
I guess it doesn't make sense to check msqid_ds.__msg_cbytes (and it's also nonstandard).

testcases/kernel/syscalls/ipc/msgctl/msgctl01.c line 83

Quoted 4 lines
testcases/kernel/syscalls/ipc/msgctl/msgctl01.c +4 -0
+	if (buf.msg_lspid == 0)
+		tst_res(TPASS, "msg_lspid = 0");
+	else
+		tst_res(TFAIL, "msg_lspid = %u", (unsigned)buf.msg_lspid);

checkpatch.pl complains about unsigned instead of unsigned int, but that's nothing we should worry about.

Kind regards,
Petr

#10 lore raw

RFC PATCH 4/9 syscalls/ipc: Rewrite msgctl02 to new library.

In reply to Cyril Hrubis
---
 testcases/kernel/syscalls/ipc/msgctl/msgctl02.c | 207 +++++++-----------------
 1 file changed, 58 insertions(+), 149 deletions(-)
testcases/kernel/syscalls/ipc/msgctl/msgctl02.c +58 -149
index 1b3909197..008fd4f71 100644
@@ -1,175 +1,84 @@
 /*
+ * Copyright (c) International Business Machines  Corp., 2001
+ *    03/2001 - Written by Wayne Boyer
+ * Copyright (c) 2018 Cyril Hrubis <[email protected]>
  *
- *   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 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.
  *
- *   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
+ * 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
  */
-
 /*
- * NAME
- *	msgctl02.c
- *
- * DESCRIPTION
- *	msgctl02 - create a message queue, then issue the IPC_SET command
- *		   to lower the msg_qbytes value.
- *
- * ALGORITHM
- *	create a message queue
- *	loop if that option was specified
- *	call msgctl() with the IPC_SET command with a new msg_qbytes value
- *	check the return code
- *	  if failure, issue a FAIL message and break remaining tests
- *	otherwise,
- *	  if doing functionality testing
- *	  	if the msg_qbytes value is the new value
- *			issue a PASS message
- *		otherwise
- *			issue a FAIL message
- *	  else issue a PASS message
- *	call cleanup
- *
- * USAGE:  <for command-line>
- *  msgctl02 [-c n] [-f] [-i n] [-I x] [-P x] [-t]
- *     where,  -c n : Run n copies concurrently.
- *             -f   : Turn off functionality Testing.
- *	       -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
- *	03/2001 - Written by Wayne Boyer
- *
- * RESTRICTIONS
- *	none
+ * Create a message queue, then issue the IPC_SET command to lower the
+ * msg_qbytes value.
  */
 
-#include "test.h"
+#include <errno.h>
 
-#include "ipcmsg.h"
+#include "tst_test.h"
+#include "tst_safe_sysv_ipc.h"
+#include "libnewipc.h"
 
-char *TCID = "msgctl02";
-int TST_TOTAL = 1;
+static int msg_id = -1;
+struct msqid_ds orig_buf;
 
-int msg_q_1 = -1;		/* to hold the message queue id */
-
-struct msqid_ds qs_buf;
-
-unsigned long int new_bytes;
-
-int main(int ac, char **av)
+static void verify_msgctl(void)
 {
-	int lc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();		/* global setup */
-
-	/* The following loop checks looping state if -i option given */
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		/* reset tst_count in case we are looping */
-		tst_count = 0;
-
-		/*
-		 * Set the msqid_ds structure values for the queue
-		 */
-
-		TEST(msgctl(msg_q_1, IPC_SET, &qs_buf));
-
-		if (TEST_RETURN == -1) {
-			tst_resm(TFAIL | TTERRNO, "msgctl() call failed");
-		} else {
-			/* do a stat to get current queue values */
-			if ((msgctl(msg_q_1, IPC_STAT, &qs_buf) == -1)) {
-				tst_resm(TBROK, "stat on queue failed");
-				continue;
-			}
-
-			if (qs_buf.msg_qbytes == new_bytes) {
-				tst_resm(TPASS, "qs_buf.msg_qbytes is"
-					 " the new value - %ld",
-					 qs_buf.msg_qbytes);
-			} else {
-				tst_resm(TFAIL, "qs_buf.msg_qbytes "
-					 "value is not expected");
-				tst_resm(TINFO, "expected - %ld, "
-					 "received - %ld", new_bytes,
-					 qs_buf.msg_qbytes);
-			}
-		}
-
-		/*
-		 * decrement by one the msq_qbytes value
-		 */
-		qs_buf.msg_qbytes -= 1;
-		new_bytes = qs_buf.msg_qbytes;
-	}
+	struct msqid_ds buf = orig_buf;
 
-	cleanup();
+	buf.msg_qbytes -= 1;
 
-	tst_exit();
-}
+	TEST(msgctl(msg_id, IPC_SET, &buf));
 
-/*
- * setup() - performs all the ONE TIME setup for this test.
- */
-void setup(void)
-{
-
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
-
-	TEST_PAUSE;
-
-	/*
-	 * Create a temporary directory and cd into it.
-	 * This helps to ensure that a unique msgkey is created.
-	 * See ../lib/libipc.c for more information.
-	 */
-	tst_tmpdir();
-
-	/* get a message key */
-	msgkey = getipckey();
+	if (TEST_RETURN != 0) {
+		tst_res(TFAIL | TTERRNO, "msgctl(IPC_SET) failed");
+		return;
+	}
 
-	/* make sure the initial # of bytes is 0 in our buffer */
-	qs_buf.msg_qbytes = 0x3000;
+	tst_res(TPASS, "msgctl(IPC_SET) msg_qbytes - 1");
 
-	/* now we have a key, so let's create a message queue */
-	if ((msg_q_1 = msgget(msgkey, IPC_CREAT | IPC_EXCL | MSG_RW)) == -1) {
-		tst_brkm(TBROK, cleanup, "Can't create message queue");
-	}
+	memset(&buf, 0, sizeof(buf));
+	SAFE_MSGCTL(msg_id, IPC_STAT, &buf);
 
-	/* now stat the queue to get the default msg_qbytes value */
-	if ((msgctl(msg_q_1, IPC_STAT, &qs_buf)) == -1) {
-		tst_brkm(TBROK, cleanup, "Can't stat the message queue");
+	if (buf.msg_qbytes == orig_buf.msg_qbytes - 1) {
+		tst_res(TPASS, "msg_qbytes = %lu",
+			(unsigned long)buf.msg_qbytes);
+	} else {
+		tst_res(TFAIL, "msg_qbytes = %lu, expected %lu",
+			(unsigned long)buf.msg_qbytes,
+			(unsigned long)orig_buf.msg_qbytes - 1);
 	}
 
-	/* decrement msg_qbytes and copy its value */
-	qs_buf.msg_qbytes -= 1;
-	new_bytes = qs_buf.msg_qbytes;
+	SAFE_MSGCTL(msg_id, IPC_SET, &orig_buf);
 }
 
-/*
- * cleanup() - performs all the ONE TIME cleanup for this test at completion
- * 	       or premature exit.
- */
-void cleanup(void)
+static void setup(void)
 {
-	/* if it exists, remove the message queue */
-	rm_queue(msg_q_1);
+	key_t msgkey = GETIPCKEY();
+
+	msg_id = SAFE_MSGGET(msgkey, IPC_CREAT | IPC_EXCL | MSG_RW | 0660);
 
-	tst_rmdir();
+	SAFE_MSGCTL(msg_id, IPC_STAT, &orig_buf);
+}
 
+static void cleanup(void)
+{
+	if (msg_id > 0)
+		SAFE_MSGCTL(msg_id, IPC_RMID, NULL);
 }
+
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = verify_msgctl,
+	.needs_tmpdir = 1
+};
Signature · 1 line
2.13.6
#12 lore raw

RFC PATCH 5/9 syscalls/ipc: Convert to newlib and merge msgctl{13, 03}

In reply to Cyril Hrubis

These two tests were both testing IPC_RMID, so I've merged then into msgctl03 and rewritten the test to new test library.

---
 runtest/syscalls                                |   1 -
 runtest/syscalls-ipc                            |   1 -
 testcases/kernel/syscalls/ipc/msgctl/.gitignore |   1 -
 testcases/kernel/syscalls/ipc/msgctl/msgctl03.c | 159 ++++++------------------
 testcases/kernel/syscalls/ipc/msgctl/msgctl13.c |  86 -------------
 5 files changed, 35 insertions(+), 213 deletions(-)
 delete mode 100644 testcases/kernel/syscalls/ipc/msgctl/msgctl13.c
runtest/syscalls +0 -1
index 7befd5c66..6b3133776 100644
@@ -696,7 +696,6 @@ msgstress02 msgstress02
 msgstress03 msgstress03
 msgstress04 msgstress04
 msgctl12 msgctl12
-msgctl13 msgctl13
 
 msgget01 msgget01
 msgget02 msgget02
runtest/syscalls-ipc +0 -1
index 9ae91bb4c..6cf5853dd 100644
@@ -9,7 +9,6 @@ msgstress02 msgstress02
 msgstress03 msgstress03
 msgstress04 msgstress04
 msgctl12 msgctl12
-msgctl13 msgctl13
 
 msgget01 msgget01
 msgget02 msgget02
testcases/kernel/syscalls/ipc/msgctl/.gitignore +0 -1
index 257a2231b..b321f0c35 100644
@@ -5,4 +5,3 @@
 /msgctl05
 /msgctl07
 /msgctl12
-/msgctl13
testcases/kernel/syscalls/ipc/msgctl/msgctl03.c +35 -124

178 lines not on this page · raw message

testcases/kernel/syscalls/ipc/msgctl/msgctl13.c deleted +0 -86

86 lines not on this page · raw message

Signature · 1 line
2.13.6
#14 lore raw

RFC PATCH 6/9 syscalls/ipc: Rewrite + merge msgctl{04, 05}

In reply to Cyril Hrubis
---
 runtest/ltplite                                 |   1 -
 runtest/stress.part3                            |   1 -
 runtest/syscalls                                |   1 -
 runtest/syscalls-ipc                            |   1 -
 testcases/kernel/syscalls/ipc/msgctl/.gitignore |   1 -
 testcases/kernel/syscalls/ipc/msgctl/msgctl04.c | 249 ++++++++----------------
 testcases/kernel/syscalls/ipc/msgctl/msgctl05.c | 186 ------------------
 7 files changed, 82 insertions(+), 358 deletions(-)
 delete mode 100644 testcases/kernel/syscalls/ipc/msgctl/msgctl05.c
runtest/ltplite +0 -1
index cb9517883..d6d1385b4 100644
@@ -483,7 +483,6 @@ msgctl01 msgctl01
 msgctl02 msgctl02
 msgctl03 msgctl03
 msgctl04 msgctl04
-msgctl05 msgctl05
 msgctl07 msgctl07
 
 msgget01 msgget01
runtest/stress.part3 +0 -1
index e97b08456..e3c308fba 100644
@@ -399,7 +399,6 @@ msgctl01 msgctl01
 msgctl02 msgctl02
 msgctl03 msgctl03
 msgctl04 msgctl04
-msgctl05 msgctl05
 msgctl07 msgctl07
 msgstress01 msgstress01
 msgstress02 msgstress02
runtest/syscalls +0 -1
index 6b3133776..5692d3e22 100644
@@ -689,7 +689,6 @@ msgctl01 msgctl01
 msgctl02 msgctl02
 msgctl03 msgctl03
 msgctl04 msgctl04
-msgctl05 msgctl05
 msgctl07 msgctl07
 msgstress01 msgstress01
 msgstress02 msgstress02
runtest/syscalls-ipc +0 -1
index 6cf5853dd..da0d5feea 100644
@@ -2,7 +2,6 @@ msgctl01 msgctl01
 msgctl02 msgctl02
 msgctl03 msgctl03
 msgctl04 msgctl04
-msgctl05 msgctl05
 msgctl07 msgctl07
 msgstress01 msgstress01
 msgstress02 msgstress02
testcases/kernel/syscalls/ipc/msgctl/.gitignore +0 -1
index b321f0c35..4289d42dc 100644
@@ -2,6 +2,5 @@
 /msgctl02
 /msgctl03
 /msgctl04
-/msgctl05
 /msgctl07
 /msgctl12
testcases/kernel/syscalls/ipc/msgctl/msgctl04.c +82 -167

285 lines not on this page · raw message

testcases/kernel/syscalls/ipc/msgctl/msgctl05.c deleted +0 -186

186 lines not on this page · raw message

Signature · 1 line
2.13.6
#16 lore raw

RFC PATCH 7/9 syscalls/ipc: Rewrite msgctl12 to new library

In reply to Cyril Hrubis
---
 testcases/kernel/syscalls/ipc/msgctl/msgctl12.c | 84 +++++++++----------------
 1 file changed, 29 insertions(+), 55 deletions(-)
testcases/kernel/syscalls/ipc/msgctl/msgctl12.c +29 -55
index 2621f8040..46e9fdb44 100644
@@ -1,6 +1,7 @@
 /*
  * Copyright (c) 2014 Fujitsu Ltd.
  * Author: Zeng Linggang <[email protected]>
+ * Copyright (c) 2018 Cyril Hrubis <[email protected]>
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms of version 2 of the GNU General Public License as
@@ -14,87 +15,60 @@
  * with this program.
  */
 /*
- * DESCRIPTION
- *	msgctl12 - test for IPC_INFO MSG_INFO and MSG_STAT.
+ * msgctl12 - test for IPC_INFO MSG_INFO and MSG_STAT.
  */
 
 #define _GNU_SOURCE
-#include <sys/types.h>
-#include <sys/ipc.h>
-#include <sys/msg.h>
 #include <errno.h>
 
-#include "test.h"
-#include "ipcmsg.h"
+#include "tst_test.h"
+#include "tst_safe_sysv_ipc.h"
+#include "libnewipc.h"
 
 static int msg_q;
 static int index_q;
 static struct msginfo msginfo_buf;
 static struct msqid_ds msgqid_buf;
 
-static struct test_case_t {
-	int *queue_id;
-	int ipc_cmd;
+static struct tcase {
+	int *msg_id;
+	int cmd;
 	char *name;
 	void *buf;
-} test_cases[] = {
+} tc[] = {
 	{&msg_q, IPC_INFO, "IPC_INFO", &msginfo_buf},
 	{&msg_q, MSG_INFO, "MSG_INFO", &msginfo_buf},
 	{&index_q, MSG_STAT, "MSG_STAT", &msgqid_buf},
 };
 
-char *TCID = "msgctl12";
-int TST_TOTAL = ARRAY_SIZE(test_cases);
-
-int main(int argc, char *argv[])
+static void verify_msgctl(unsigned int i)
 {
-	int lc;
-	int i;
-
-	tst_parse_opts(argc, argv, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-
-		tst_count = 0;
-
-		for (i = 0; i < TST_TOTAL; i++) {
+	TEST(msgctl(*tc[i].msg_id,  tc[i].cmd, tc[i].buf));
 
-			TEST(msgctl(*test_cases[i].queue_id,
-				    test_cases[i].ipc_cmd, test_cases[i].buf));
-
-			if (TEST_RETURN == -1) {
-				tst_resm(TFAIL,
-					 "msgctl() test %s failed with errno: "
-					 "%d", test_cases[i].name, TEST_ERRNO);
-			} else {
-				tst_resm(TPASS, "msgctl() test %s succeeded",
-					 test_cases[i].name);
-			}
-		}
+	if (TEST_RETURN == -1) {
+		tst_res(TFAIL,
+			 "msgctl() test %s failed with errno: "
+			 "%d", tc[i].name, TEST_ERRNO);
 	}
 
-	cleanup();
-	tst_exit();
+	tst_res(TPASS, "msgctl() test %s succeeded", tc[i].name);
 }
 
-void setup(void)
+static void setup(void)
 {
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
-
-	TEST_PAUSE;
-
-	msg_q = msgget(IPC_PRIVATE, MSG_RW);
-	if (msg_q < 0)
-		tst_brkm(TBROK, cleanup, "Can't create message queue");
-
-	index_q = msgctl(msg_q, IPC_INFO, (struct msqid_ds *)&msginfo_buf);
-	if (index_q < 0)
-		tst_brkm(TBROK, cleanup, "Can't create message queue");
+	msg_q = SAFE_MSGGET(IPC_PRIVATE, MSG_RW);
+	index_q = SAFE_MSGCTL(msg_q, IPC_INFO, (struct msqid_ds*)&msginfo_buf);
 }
 
-void cleanup(void)
+static void cleanup(void)
 {
-	rm_queue(msg_q);
+	if (msg_q > 0)
+		SAFE_MSGCTL(msg_q, IPC_RMID, NULL);
 }
+
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.test = verify_msgctl,
+	.tcnt = ARRAY_SIZE(tc),
+};
Signature · 1 line
2.13.6
#17 lore raw

RFC PATCH 7/9 syscalls/ipc: Rewrite msgctl12 to new library

Hi Cyril,

Quoted 1 line

testcases/kernel/syscalls/ipc/msgctl/msgctl12.c line 37

Quoted 7 lines
---
 testcases/kernel/syscalls/ipc/msgctl/msgctl12.c | 84 +++++++++----------------
testcases/kernel/syscalls/ipc/msgctl/msgctl12.c +0 -3
-static struct test_case_t {
-	int *queue_id;
-	int ipc_cmd;

LGTM, I'd use macro for string, but that's only nitpicking.

#define CMD_NAME(x) .cmd = x, .name = #x

testcases/kernel/syscalls/ipc/msgctl/msgctl12.c line 41

Quoted 10 lines
testcases/kernel/syscalls/ipc/msgctl/msgctl12.c +4 -1
+static struct tcase {
+	int *msg_id;
+	int cmd;
 	char *name;
 	void *buf;
-} test_cases[] = {
+} tc[] = {
 	{&msg_q, IPC_INFO, "IPC_INFO", &msginfo_buf},
 	{&msg_q, MSG_INFO, "MSG_INFO", &msginfo_buf},
 	{&index_q, MSG_STAT, "MSG_STAT", &msgqid_buf},

+ this:

    {&msg_q, CMD_NAME(IPC_INFO), &msginfo_buf},
    {&msg_q, CMD_NAME(MSG_INFO), &msginfo_buf},
    {&index_q, CMD_NAME(MSG_STAT), &msgqid_buf},

Kind regards,
Petr

#18 lore raw

RFC PATCH 8/9 syscalls/ipc: Remove msgctl07

In reply to Cyril Hrubis

The test is very similar to msgrcv01, does not test msgctl() and the code is messed up, hence it makes sense to remove it rather than moving it to msgrcv directory.

---
 runtest/ltplite                                 |   1 -
 runtest/stress.part3                            |   1 -
 runtest/syscalls                                |   1 -
 runtest/syscalls-ipc                            |   1 -
 testcases/kernel/syscalls/ipc/msgctl/.gitignore |   1 -
 testcases/kernel/syscalls/ipc/msgctl/msgctl07.c | 292 ------------------------
 6 files changed, 297 deletions(-)
 delete mode 100644 testcases/kernel/syscalls/ipc/msgctl/msgctl07.c
runtest/ltplite +0 -1
index d6d1385b4..f06e8888a 100644
@@ -483,7 +483,6 @@ msgctl01 msgctl01
 msgctl02 msgctl02
 msgctl03 msgctl03
 msgctl04 msgctl04
-msgctl07 msgctl07
 
 msgget01 msgget01
 msgget02 msgget02
runtest/stress.part3 +0 -1
index e3c308fba..1fb2c12cc 100644
@@ -399,7 +399,6 @@ msgctl01 msgctl01
 msgctl02 msgctl02
 msgctl03 msgctl03
 msgctl04 msgctl04
-msgctl07 msgctl07
 msgstress01 msgstress01
 msgstress02 msgstress02
 
runtest/syscalls +0 -1
index 5692d3e22..4f58ec2e9 100644
@@ -689,7 +689,6 @@ msgctl01 msgctl01
 msgctl02 msgctl02
 msgctl03 msgctl03
 msgctl04 msgctl04
-msgctl07 msgctl07
 msgstress01 msgstress01
 msgstress02 msgstress02
 msgstress03 msgstress03
runtest/syscalls-ipc +0 -1
index da0d5feea..00d7eed3a 100644
@@ -2,7 +2,6 @@ msgctl01 msgctl01
 msgctl02 msgctl02
 msgctl03 msgctl03
 msgctl04 msgctl04
-msgctl07 msgctl07
 msgstress01 msgstress01
 msgstress02 msgstress02
 msgstress03 msgstress03
testcases/kernel/syscalls/ipc/msgctl/.gitignore +0 -1
index 4289d42dc..f179f2606 100644
@@ -2,5 +2,4 @@
 /msgctl02
 /msgctl03
 /msgctl04
-/msgctl07
 /msgctl12
testcases/kernel/syscalls/ipc/msgctl/msgctl07.c deleted +0 -292

292 lines not on this page · raw message

Signature · 1 line
2.13.6
#20 lore raw

RFC PATCH 9/9 syscalls/ipc: Fix Makefile

In reply to Cyril Hrubis

Switch to new ipc library now that all testcases are rewritten.

---
 testcases/kernel/syscalls/ipc/msgctl/Makefile | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
testcases/kernel/syscalls/ipc/msgctl/Makefile +1 -7
index 4472eeb1c..f9ee8d2c2 100644
@@ -19,11 +19,5 @@
 top_srcdir              ?= ../../../../..
 
 include $(top_srcdir)/include/mk/testcases.mk
-
-LIBMSGCTL               := $(LIBDIR)/libmsgctl.a
-LDLIBS                  += -lmsgctl
-
-MAKE_DEPS               := $(LIBMSGCTL)
-
-include $(abs_srcdir)/../Makefile.inc
+include $(abs_srcdir)/../Makefile2.inc
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
Signature · 1 line
2.13.6
#21 lore raw

RFC PATCH 9/9 syscalls/ipc: Fix Makefile

Hi Cyril,

Quoted 3 lines

Switch to new ipc library now that all testcases are rewritten.

Quoted 1 line

---

I've noticed, that some commits from this patchset aren't bisectable, due dependency on new library (libnewipc.h).
IMHO it's worth of fixing it.

Kind regards,
Petr

Quoted 20 lines
 testcases/kernel/syscalls/ipc/msgctl/Makefile | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
testcases/kernel/syscalls/ipc/msgctl/Makefile +1 -7
index 4472eeb1c..f9ee8d2c2 100644
@@ -19,11 +19,5 @@
 top_srcdir              ?= ../../../../..
 
 include $(top_srcdir)/include/mk/testcases.mk
-
-LIBMSGCTL               := $(LIBDIR)/libmsgctl.a
-LDLIBS                  += -lmsgctl
-
-MAKE_DEPS               := $(LIBMSGCTL)
-
-include $(abs_srcdir)/../Makefile.inc
+include $(abs_srcdir)/../Makefile2.inc
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
#22 lore raw

RFC PATCH 9/9 syscalls/ipc: Fix Makefile

Hi!

Quoted 3 lines

I've noticed, that some commits from this patchset aren't bisectable, due dependency on new library (libnewipc.h).
IMHO it's worth of fixing it.

I guess that the best I can do is to disable the directory from build before the patchset and enable it with a new Makefile at the end as it's nearly impossible to mix the new and old IPC library in one Makefile.

And yes I wanted to send this early, so I kind of ignored the build problems.

Signature · 2 lines
Cyril Hrubis
[email protected]
#23 lore raw

RFC PATCH 9/9 syscalls/ipc: Fix Makefile

Hi Cyril,

Quoted 8 lines

Hi!

Quoted 3 lines

I've noticed, that some commits from this patchset aren't bisectable, due dependency on new library (libnewipc.h).
IMHO it's worth of fixing it.

I guess that the best I can do is to disable the directory from build before the patchset and enable it with a new Makefile at the end as it's nearly impossible to mix the new and old IPC library in one Makefile.

Seems reasonable for me.
Thanks for handling it.

Quoted 2 lines

And yes I wanted to send this early, so I kind of ignored the build problems.

No big deal :).

Kind regards,
Petr