[LTP] [PATCH] syscalls/msgrcv07: Add functional test for MSG_COPY flag

[LTP] [PATCH] syscalls/msgrcv07: Add functional test for MSG_COPY flag

[Yang Xu]

When specifying MSG_COPY flag, we can read the msg but don't destory
it in msg queue and mtype is interpreted as number of the message to
copy. We check the read data whether correctly and use msgctl to
check whether we still have 2 msg in msg queue after msgrcv(MSG_COPY).

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
Hi Cyril
This patch is based on new msgrcv07.c(test different msgtyp) and also
based on MSG_COPY error test(introduced MSG_COPY flag in lapi/msg.h).
 .../kernel/syscalls/ipc/msgrcv/msgrcv07.c     | 63 +++++++++++++++++--
 1 file changed, 59 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c b/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c
index f6139ba57..51dbc8951 100644
--- a/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c
+++ b/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c
@@ -3,12 +3,13 @@
  * Copyright (c) 2014-2020 Fujitsu Ltd.
  * Author: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
  *
- * Basic test for msgrcv(2) using MSG_EXCEPT, MSG_NOERROR and different
- * msg_typ(zero,positive,negative).
+ * Basic test for msgrcv(2) using MSG_EXCEPT, MSG_NOERROR, MSG_COPY
+ * and different msg_typ(zero,positive,negative).
  */
 
 #define  _GNU_SOURCE
 #include <sys/wait.h>
+#include "lapi/msg.h"
 #include "tst_test.h"
 #include "tst_safe_sysv_ipc.h"
 #include "libnewipc.h"
@@ -19,7 +20,7 @@
 #define MSG2	"messagetype2"
 
 static key_t msgkey;
-static int queue_id = -1;
+static int queue_id = -1, msg_copy_sup;
 static struct buf {
 	long type;
 	char mtext[MSGSIZE];
@@ -141,15 +142,69 @@ static void test_negative_msgtyp(void)
 	SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
 }
 
+static void test_msg_copy(void)
+{
+	struct msqid_ds buf = {0};
+
+	if (!msg_copy_sup)
+		tst_res(TCONF, "kernel doesn't support MSG_COPY flag, skip it");
+	prepare_queue();
+
+	/*
+	 * If MSG_COPY flag was specified, then mtype is interpreted as number
+	 * of the message to copy.
+	 */
+	SAFE_MSGRCV(queue_id, &rcv_buf, MSGSIZE, 0, MSG_COPY | IPC_NOWAIT);
+	if (strcmp(rcv_buf.mtext, MSG1) == 0 && rcv_buf.type == MSGTYPE1)
+		tst_res(TPASS, "msgrcv(MSG_COPY) got MSGTYPE1 msg data"
+			" correctly");
+	else
+		tst_res(TFAIL, "msgrcv(MSG_COPY) got MSGTYPE1 msg data"
+			" incorrectly");
+
+	SAFE_MSGRCV(queue_id, &rcv_buf, MSGSIZE, 1, MSG_COPY | IPC_NOWAIT);
+	if (strcmp(rcv_buf.mtext, MSG2) == 0 && rcv_buf.type == MSGTYPE2)
+		tst_res(TPASS, "msgrcv(MSG_COPY) got MSGTYPE2 msg data"
+			" correctly");
+	else
+		tst_res(TFAIL, "msgrcv(MSG_COPY) got MSGTYPE2 msg data"
+			" incorrectly");
+
+	SAFE_MSGCTL(queue_id, IPC_STAT, &buf);
+	if (buf.msg_qnum == 2)
+		tst_res(TPASS, "msgrcv(MSG_COPY) succeeded, msg queue "
+			"still has 2 msg");
+	else
+		tst_res(TFAIL, "msgrcv(MSG_COPY) msg queue expected 2 msg num,"
+			" but only got %d", (int)buf.msg_qnum);
+	SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
+}
 
 static void setup(void)
 {
 	msgkey = GETIPCKEY();
+	prepare_queue();
+	TEST(msgrcv(queue_id, &rcv_buf, MSGSIZE, MSGTYPE1, MSG_COPY));
+	if (TST_RET != -1)
+		tst_res(TINFO, "msgrcv succeeded unexpectedly, kernel doesn't"
+			" support MSG_COPY flag");
+
+	if (TST_ERR == EINVAL) {
+		tst_res(TINFO, "msgrcv failed as expected when not using"
+			" MSG_COPY and IPC_NOWAIT concurrently");
+		msg_copy_sup = 1;
+	} else if (TST_ERR == ENOSYS) {
+		tst_res(TINFO, "kernel doesn't enable CONFIG_CHECKPOINT_RESTORE");
+	} else {
+		tst_res(TINFO | TTERRNO, "msgrcv failed when not using MSG_COPY"
+			"and IPC_NOWAIT concurrently, expected EINVAL but got");
+	}
+	cleanup();
 }
 
 static void (*testfunc[])(void) = {test_msg_except, test_msg_noerror,
 				   test_zero_msgtyp, test_positive_msgtyp,
-				   test_negative_msgtyp};
+				   test_negative_msgtyp, test_msg_copy};
 
 static void verify_msgcrv(unsigned int n)
 {
-- 
2.23.0

[LTP] [PATCH] syscalls/msgrcv07: Add functional test for MSG_COPY flag

[Yang Xu]

Hi Cyril

> When specifying MSG_COPY flag, we can read the msg but don't destory
> it in msg queue and mtype is interpreted as number of the message to
> copy. We check the read data whether correctly and use msgctl to
> check whether we still have 2 msg in msg queue after msgrcv(MSG_COPY).
> 
> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> ---
> Hi Cyril
> This patch is based on new msgrcv07.c(test different msgtyp) and also
> based on MSG_COPY error test(introduced MSG_COPY flag in lapi/msg.h).
>   .../kernel/syscalls/ipc/msgrcv/msgrcv07.c     | 63 +++++++++++++++++--
>   1 file changed, 59 insertions(+), 4 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c b/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c
> index f6139ba57..51dbc8951 100644
> --- a/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c
> +++ b/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c
> @@ -3,12 +3,13 @@
>    * Copyright (c) 2014-2020 Fujitsu Ltd.
>    * Author: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
>    *
> - * Basic test for msgrcv(2) using MSG_EXCEPT, MSG_NOERROR and different
> - * msg_typ(zero,positive,negative).
> + * Basic test for msgrcv(2) using MSG_EXCEPT, MSG_NOERROR, MSG_COPY
> + * and different msg_typ(zero,positive,negative).
>    */
>   
>   #define  _GNU_SOURCE
>   #include <sys/wait.h>
> +#include "lapi/msg.h"
>   #include "tst_test.h"
>   #include "tst_safe_sysv_ipc.h"
>   #include "libnewipc.h"
> @@ -19,7 +20,7 @@
>   #define MSG2	"messagetype2"
>   
>   static key_t msgkey;
> -static int queue_id = -1;
> +static int queue_id = -1, msg_copy_sup;
>   static struct buf {
>   	long type;
>   	char mtext[MSGSIZE];
> @@ -141,15 +142,69 @@ static void test_negative_msgtyp(void)
>   	SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
>   }
>   
> +static void test_msg_copy(void)
> +{
> +	struct msqid_ds buf = {0};
> +
> +	if (!msg_copy_sup)
> +		tst_res(TCONF, "kernel doesn't support MSG_COPY flag, skip it");
Sorry, here missed a return;
> +	prepare_queue();
> +
> +	/*
> +	 * If MSG_COPY flag was specified, then mtype is interpreted as number
> +	 * of the message to copy.
> +	 */
> +	SAFE_MSGRCV(queue_id, &rcv_buf, MSGSIZE, 0, MSG_COPY | IPC_NOWAIT);
> +	if (strcmp(rcv_buf.mtext, MSG1) == 0 && rcv_buf.type == MSGTYPE1)
> +		tst_res(TPASS, "msgrcv(MSG_COPY) got MSGTYPE1 msg data"
> +			" correctly");
> +	else
> +		tst_res(TFAIL, "msgrcv(MSG_COPY) got MSGTYPE1 msg data"
> +			" incorrectly");
> +
> +	SAFE_MSGRCV(queue_id, &rcv_buf, MSGSIZE, 1, MSG_COPY | IPC_NOWAIT);
> +	if (strcmp(rcv_buf.mtext, MSG2) == 0 && rcv_buf.type == MSGTYPE2)
> +		tst_res(TPASS, "msgrcv(MSG_COPY) got MSGTYPE2 msg data"
> +			" correctly");
> +	else
> +		tst_res(TFAIL, "msgrcv(MSG_COPY) got MSGTYPE2 msg data"
> +			" incorrectly");
> +
> +	SAFE_MSGCTL(queue_id, IPC_STAT, &buf);
> +	if (buf.msg_qnum == 2)
> +		tst_res(TPASS, "msgrcv(MSG_COPY) succeeded, msg queue "
> +			"still has 2 msg");
> +	else
> +		tst_res(TFAIL, "msgrcv(MSG_COPY) msg queue expected 2 msg num,"
> +			" but only got %d", (int)buf.msg_qnum);
> +	SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
> +}
>   
>   static void setup(void)
>   {
>   	msgkey = GETIPCKEY();
> +	prepare_queue();
> +	TEST(msgrcv(queue_id, &rcv_buf, MSGSIZE, MSGTYPE1, MSG_COPY));
> +	if (TST_RET != -1)
> +		tst_res(TINFO, "msgrcv succeeded unexpectedly, kernel doesn't"
> +			" support MSG_COPY flag");
> +
> +	if (TST_ERR == EINVAL) {
> +		tst_res(TINFO, "msgrcv failed as expected when not using"
> +			" MSG_COPY and IPC_NOWAIT concurrently");
> +		msg_copy_sup = 1;
> +	} else if (TST_ERR == ENOSYS) {
> +		tst_res(TINFO, "kernel doesn't enable CONFIG_CHECKPOINT_RESTORE");
> +	} else {
> +		tst_res(TINFO | TTERRNO, "msgrcv failed when not using MSG_COPY"
> +			"and IPC_NOWAIT concurrently, expected EINVAL but got");
> +	}
> +	cleanup();
>   }
>   
>   static void (*testfunc[])(void) = {test_msg_except, test_msg_noerror,
>   				   test_zero_msgtyp, test_positive_msgtyp,
> -				   test_negative_msgtyp};
> +				   test_negative_msgtyp, test_msg_copy};
>   
>   static void verify_msgcrv(unsigned int n)
>   {
> 

[LTP] [PATCH] syscalls/msgrcv07: Add functional test for MSG_COPY flag

[Cyril Hrubis]

Hi!
> +static void test_msg_copy(void)
> +{
> +	struct msqid_ds buf = {0};
> +
> +	if (!msg_copy_sup)
> +		tst_res(TCONF, "kernel doesn't support MSG_COPY flag, skip it");
> +	prepare_queue();
> +
> +	/*
> +	 * If MSG_COPY flag was specified, then mtype is interpreted as number
> +	 * of the message to copy.
> +	 */
> +	SAFE_MSGRCV(queue_id, &rcv_buf, MSGSIZE, 0, MSG_COPY | IPC_NOWAIT);
> +	if (strcmp(rcv_buf.mtext, MSG1) == 0 && rcv_buf.type == MSGTYPE1)
> +		tst_res(TPASS, "msgrcv(MSG_COPY) got MSGTYPE1 msg data"
> +			" correctly");
> +	else
> +		tst_res(TFAIL, "msgrcv(MSG_COPY) got MSGTYPE1 msg data"
> +			" incorrectly");
> +
> +	SAFE_MSGRCV(queue_id, &rcv_buf, MSGSIZE, 1, MSG_COPY | IPC_NOWAIT);
> +	if (strcmp(rcv_buf.mtext, MSG2) == 0 && rcv_buf.type == MSGTYPE2)
> +		tst_res(TPASS, "msgrcv(MSG_COPY) got MSGTYPE2 msg data"
> +			" correctly");
> +	else
> +		tst_res(TFAIL, "msgrcv(MSG_COPY) got MSGTYPE2 msg data"
> +			" incorrectly");
> +
> +	SAFE_MSGCTL(queue_id, IPC_STAT, &buf);
> +	if (buf.msg_qnum == 2)
> +		tst_res(TPASS, "msgrcv(MSG_COPY) succeeded, msg queue "
> +			"still has 2 msg");
> +	else
> +		tst_res(TFAIL, "msgrcv(MSG_COPY) msg queue expected 2 msg num,"
> +			" but only got %d", (int)buf.msg_qnum);
> +	SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
> +}
>  
>  static void setup(void)
>  {
>  	msgkey = GETIPCKEY();
> +	prepare_queue();
> +	TEST(msgrcv(queue_id, &rcv_buf, MSGSIZE, MSGTYPE1, MSG_COPY));
> +	if (TST_RET != -1)
> +		tst_res(TINFO, "msgrcv succeeded unexpectedly, kernel doesn't"
> +			" support MSG_COPY flag");
> +
> +	if (TST_ERR == EINVAL) {
> +		tst_res(TINFO, "msgrcv failed as expected when not using"
> +			" MSG_COPY and IPC_NOWAIT concurrently");
> +		msg_copy_sup = 1;
> +	} else if (TST_ERR == ENOSYS) {
> +		tst_res(TINFO, "kernel doesn't enable CONFIG_CHECKPOINT_RESTORE");
> +	} else {
> +		tst_res(TINFO | TTERRNO, "msgrcv failed when not using MSG_COPY"
> +			"and IPC_NOWAIT concurrently, expected EINVAL but got");
> +	}

Why can't we just check for ENOSYS in the test_msg_copy() instead?

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] syscalls/msgrcv07: Add functional test for MSG_COPY flag

[Yang Xu]

Hi Cyril

> Hi!
>> +static void test_msg_copy(void)
>> +{
>> +	struct msqid_ds buf = {0};
>> +
>> +	if (!msg_copy_sup)
>> +		tst_res(TCONF, "kernel doesn't support MSG_COPY flag, skip it");
>> +	prepare_queue();
>> +
>> +	/*
>> +	 * If MSG_COPY flag was specified, then mtype is interpreted as number
>> +	 * of the message to copy.
>> +	 */
>> +	SAFE_MSGRCV(queue_id, &rcv_buf, MSGSIZE, 0, MSG_COPY | IPC_NOWAIT);
>> +	if (strcmp(rcv_buf.mtext, MSG1) == 0 && rcv_buf.type == MSGTYPE1)
>> +		tst_res(TPASS, "msgrcv(MSG_COPY) got MSGTYPE1 msg data"
>> +			" correctly");
>> +	else
>> +		tst_res(TFAIL, "msgrcv(MSG_COPY) got MSGTYPE1 msg data"
>> +			" incorrectly");
>> +
>> +	SAFE_MSGRCV(queue_id, &rcv_buf, MSGSIZE, 1, MSG_COPY | IPC_NOWAIT);
>> +	if (strcmp(rcv_buf.mtext, MSG2) == 0 && rcv_buf.type == MSGTYPE2)
>> +		tst_res(TPASS, "msgrcv(MSG_COPY) got MSGTYPE2 msg data"
>> +			" correctly");
>> +	else
>> +		tst_res(TFAIL, "msgrcv(MSG_COPY) got MSGTYPE2 msg data"
>> +			" incorrectly");
>> +
>> +	SAFE_MSGCTL(queue_id, IPC_STAT, &buf);
>> +	if (buf.msg_qnum == 2)
>> +		tst_res(TPASS, "msgrcv(MSG_COPY) succeeded, msg queue "
>> +			"still has 2 msg");
>> +	else
>> +		tst_res(TFAIL, "msgrcv(MSG_COPY) msg queue expected 2 msg num,"
>> +			" but only got %d", (int)buf.msg_qnum);
>> +	SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
>> +}
>>   
>>   static void setup(void)
>>   {
>>   	msgkey = GETIPCKEY();
>> +	prepare_queue();
>> +	TEST(msgrcv(queue_id, &rcv_buf, MSGSIZE, MSGTYPE1, MSG_COPY));
>> +	if (TST_RET != -1)
>> +		tst_res(TINFO, "msgrcv succeeded unexpectedly, kernel doesn't"
>> +			" support MSG_COPY flag");
>> +
>> +	if (TST_ERR == EINVAL) {
>> +		tst_res(TINFO, "msgrcv failed as expected when not using"
>> +			" MSG_COPY and IPC_NOWAIT concurrently");
>> +		msg_copy_sup = 1;
>> +	} else if (TST_ERR == ENOSYS) {
>> +		tst_res(TINFO, "kernel doesn't enable CONFIG_CHECKPOINT_RESTORE");
>> +	} else {
>> +		tst_res(TINFO | TTERRNO, "msgrcv failed when not using MSG_COPY"
>> +			"and IPC_NOWAIT concurrently, expected EINVAL but got");
>> +	}
> 
> Why can't we just check for ENOSYS in the test_msg_copy() instead?
In here, ENOSYS is differnt from other syscalls. only specifying 
MSG_COPY flag and CONFIG_CHECKPOINT_RESTORE was not enabled, returns 
ENOSYS. See[1]

So I use MSG_COPY without IPC_NOWAIT to check whether kernel supports 
MSG_COPY flag(It will trigger EINVAL error as man-page said).

If I misunderstand you, please let me know.

[1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4a674f34ba04a0

> 

[LTP] [PATCH v2] syscalls/msgrcv07: Add functional test for MSG_COPY flag

[Yang Xu]

When specifying MSG_COPY flag, we can read the msg but don't destory
it in msg queue and mtype is interpreted as number of the message to
copy. We check the read data whether correctly and use msgctl to
check whether we still have 2 msg in msg queue after msgrcv(MSG_COPY).

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
v1->v2:
1.add missing return when not support MSG_COPY flag
2.modify the test order(also rebase this patch)
 .../kernel/syscalls/ipc/msgrcv/msgrcv07.c     | 71 +++++++++++++++++--
 1 file changed, 66 insertions(+), 5 deletions(-)

diff --git a/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c b/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c
index 8005a9acd..3e8b382a9 100644
--- a/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c
+++ b/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c
@@ -2,9 +2,10 @@
 /*
  * Copyright (c) 2014-2020 Fujitsu Ltd.
  * Author: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
+ * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
  *
- * Basic test for msgrcv(2) using MSG_EXCEPT, MSG_NOERROR and different
- * msg_typ(zero,positive,negative).
+ * Basic test for msgrcv(2) using MSG_EXCEPT, MSG_NOERROR, MSG_COPY and
+ * different msg_typ(zero,positive,negative).
  *
  * * With MSG_EXCEPT flag any message type but the one passed to the function
  *   is received.
@@ -12,6 +13,9 @@
  * * With MSG_NOERROR and buffer size less than message size only part of the
  *   buffer is received.
  *
+ * * With MSG_COPY and IPC_NOWAIT flag read the msg but don't destroy it in
+ *   msg queue.
+ *
  * * With msgtyp is 0, then the first message in the queue is read.
  *
  * * With msgtyp is greater than 0, then the first message in the queue of type
@@ -33,7 +37,7 @@
 #define MSG2	"messagetype2"
 
 static key_t msgkey;
-static int queue_id = -1;
+static int queue_id = -1, msg_copy_sup;
 static struct buf {
 	long type;
 	char mtext[MSGSIZE];
@@ -97,6 +101,46 @@ static void test_msg_noerror(void)
 	SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
 }
 
+static void test_msg_copy(void)
+{
+	struct msqid_ds buf = {0};
+
+	if (!msg_copy_sup) {
+		tst_res(TCONF, "kernel doesn't support MSG_COPY flag, skip it");
+		return;
+	}
+	prepare_queue();
+
+	/*
+	 * If MSG_COPY flag was specified, then mtype is interpreted as number
+	 * of the message to copy.
+	 */
+	SAFE_MSGRCV(queue_id, &rcv_buf, MSGSIZE, 0, MSG_COPY | IPC_NOWAIT);
+	if (strcmp(rcv_buf.mtext, MSG1) == 0 && rcv_buf.type == MSGTYPE1)
+		tst_res(TPASS, "msgrcv(MSG_COPY) got MSGTYPE1 msg data"
+			" correctly");
+	else
+		tst_res(TFAIL, "msgrcv(MSG_COPY) got MSGTYPE1 msg data"
+			" incorrectly");
+
+	SAFE_MSGRCV(queue_id, &rcv_buf, MSGSIZE, 1, MSG_COPY | IPC_NOWAIT);
+	if (strcmp(rcv_buf.mtext, MSG2) == 0 && rcv_buf.type == MSGTYPE2)
+		tst_res(TPASS, "msgrcv(MSG_COPY) got MSGTYPE2 msg data"
+			" correctly");
+	else
+		tst_res(TFAIL, "msgrcv(MSG_COPY) got MSGTYPE2 msg data"
+			" incorrectly");
+
+	SAFE_MSGCTL(queue_id, IPC_STAT, &buf);
+	if (buf.msg_qnum == 2)
+		tst_res(TPASS, "msgrcv(MSG_COPY) succeeded, msg queue "
+			"still has 2 msg");
+	else
+		tst_res(TFAIL, "msgrcv(MSG_COPY) msg queue expected 2 msg num,"
+			" but only got %d", (int)buf.msg_qnum);
+	SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
+}
+
 static void test_zero_msgtyp(void)
 {
 	prepare_queue();
@@ -159,11 +203,28 @@ static void test_negative_msgtyp(void)
 static void setup(void)
 {
 	msgkey = GETIPCKEY();
+	prepare_queue();
+	TEST(msgrcv(queue_id, &rcv_buf, MSGSIZE, MSGTYPE1, MSG_COPY));
+	if (TST_RET != -1)
+		tst_res(TINFO, "msgrcv succeeded unexpectedly, kernel doesn't"
+			" support MSG_COPY flag");
+
+	if (TST_ERR == EINVAL) {
+		tst_res(TINFO, "msgrcv failed as expected when not using"
+			" MSG_COPY and IPC_NOWAIT concurrently");
+		msg_copy_sup = 1;
+	} else if (TST_ERR == ENOSYS) {
+		tst_res(TINFO, "kernel doesn't enable CONFIG_CHECKPOINT_RESTORE");
+	} else {
+		tst_res(TINFO | TTERRNO, "msgrcv failed when not using MSG_COPY"
+			"and IPC_NOWAIT concurrently, expected EINVAL but got");
+	}
+	cleanup();
 }
 
 static void (*testfunc[])(void) = {test_msg_except, test_msg_noerror,
-				   test_zero_msgtyp, test_positive_msgtyp,
-				   test_negative_msgtyp};
+				   test_msg_copy, test_zero_msgtyp,
+				   test_positive_msgtyp, test_negative_msgtyp};
 
 static void verify_msgcrv(unsigned int n)
 {
-- 
2.23.0

[LTP] [PATCH v2] syscalls/msgrcv07: Add functional test for MSG_COPY flag

[Cyril Hrubis]

Hi!
> +static void test_msg_copy(void)
> +{
> +	struct msqid_ds buf = {0};
> +
> +	if (!msg_copy_sup) {
> +		tst_res(TCONF, "kernel doesn't support MSG_COPY flag, skip it");
> +		return;
> +	}
> +	prepare_queue();
> +
> +	/*
> +	 * If MSG_COPY flag was specified, then mtype is interpreted as number
> +	 * of the message to copy.
> +	 */
> +	SAFE_MSGRCV(queue_id, &rcv_buf, MSGSIZE, 0, MSG_COPY | IPC_NOWAIT);
> +	if (strcmp(rcv_buf.mtext, MSG1) == 0 && rcv_buf.type == MSGTYPE1)
> +		tst_res(TPASS, "msgrcv(MSG_COPY) got MSGTYPE1 msg data"
> +			" correctly");
> +	else
> +		tst_res(TFAIL, "msgrcv(MSG_COPY) got MSGTYPE1 msg data"
> +			" incorrectly");
> +
> +	SAFE_MSGRCV(queue_id, &rcv_buf, MSGSIZE, 1, MSG_COPY | IPC_NOWAIT);
> +	if (strcmp(rcv_buf.mtext, MSG2) == 0 && rcv_buf.type == MSGTYPE2)
> +		tst_res(TPASS, "msgrcv(MSG_COPY) got MSGTYPE2 msg data"
> +			" correctly");
> +	else
> +		tst_res(TFAIL, "msgrcv(MSG_COPY) got MSGTYPE2 msg data"
> +			" incorrectly");

Can we please keep the strings on a single line?

I guess that we can shorten the messages a bit e.g.

tst_res(TFAIL, "msgrcv(MSG_COPY) got MSGTYPE2 data correctly");

Or even:

tst_res(TFAIL, "MSG_COPY got MSGTYPE2 data correctly");

Please try to keep the messages short and to the point.

> +	SAFE_MSGCTL(queue_id, IPC_STAT, &buf);
> +	if (buf.msg_qnum == 2)
> +		tst_res(TPASS, "msgrcv(MSG_COPY) succeeded, msg queue "
> +			"still has 2 msg");
> +	else
> +		tst_res(TFAIL, "msgrcv(MSG_COPY) msg queue expected 2 msg num,"
> +			" but only got %d", (int)buf.msg_qnum);

Here as well we can shorten it to something as:

	tst_res(TPASS, "Two messages still in queue");

	tst_res(TFAIL, "Expected 2 msgs in queue got %i",
		(int)buf.msg_qnum);

> +	SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
> +}
> +
>  static void test_zero_msgtyp(void)
>  {
>  	prepare_queue();
> @@ -159,11 +203,28 @@ static void test_negative_msgtyp(void)
>  static void setup(void)
>  {
>  	msgkey = GETIPCKEY();
> +	prepare_queue();
> +	TEST(msgrcv(queue_id, &rcv_buf, MSGSIZE, MSGTYPE1, MSG_COPY));
> +	if (TST_RET != -1)
> +		tst_res(TINFO, "msgrcv succeeded unexpectedly, kernel doesn't"
> +			" support MSG_COPY flag");
> +
> +	if (TST_ERR == EINVAL) {
> +		tst_res(TINFO, "msgrcv failed as expected when not using"
> +			" MSG_COPY and IPC_NOWAIT concurrently");
> +		msg_copy_sup = 1;
> +	} else if (TST_ERR == ENOSYS) {
> +		tst_res(TINFO, "kernel doesn't enable CONFIG_CHECKPOINT_RESTORE");
> +	} else {
> +		tst_res(TINFO | TTERRNO, "msgrcv failed when not using MSG_COPY"
> +			"and IPC_NOWAIT concurrently, expected EINVAL but got");
> +	}

This check actually does not work, it fails to detect the support on
newer kernels, e.g. 5.6.1 without the CONFIG_CHECKPOINT_RESTORE.

When kernel is new enough call to msgrcv() with MSG_COPY without
IPC_NOWAIT returns EINVAL even without CONFIG_CHECKPOINT_RESTORE I guess
that the flags are checked in the generic code and not ifdefed out. You
will get ENOSYS only on correct combination i.e. MSG_COPY | IPC_NOWAIT.

So why don't we rather:

* Check if the kernel is older than 3.8 and skip the MSG_COPY test when
  it is

* Check for ENOSYS in the test_msg_copy() function

These two conditions should cover all possible cases.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2] syscalls/msgrcv07: Add functional test for MSG_COPY flag

[Yang Xu]

Hi Cyril

> Hi!
>> +static void test_msg_copy(void)
>> +{
>> +	struct msqid_ds buf = {0};
>> +
>> +	if (!msg_copy_sup) {
>> +		tst_res(TCONF, "kernel doesn't support MSG_COPY flag, skip it");
>> +		return;
>> +	}
>> +	prepare_queue();
>> +
>> +	/*
>> +	 * If MSG_COPY flag was specified, then mtype is interpreted as number
>> +	 * of the message to copy.
>> +	 */
>> +	SAFE_MSGRCV(queue_id, &rcv_buf, MSGSIZE, 0, MSG_COPY | IPC_NOWAIT);
>> +	if (strcmp(rcv_buf.mtext, MSG1) == 0 && rcv_buf.type == MSGTYPE1)
>> +		tst_res(TPASS, "msgrcv(MSG_COPY) got MSGTYPE1 msg data"
>> +			" correctly");
>> +	else
>> +		tst_res(TFAIL, "msgrcv(MSG_COPY) got MSGTYPE1 msg data"
>> +			" incorrectly");
>> +
>> +	SAFE_MSGRCV(queue_id, &rcv_buf, MSGSIZE, 1, MSG_COPY | IPC_NOWAIT);
>> +	if (strcmp(rcv_buf.mtext, MSG2) == 0 && rcv_buf.type == MSGTYPE2)
>> +		tst_res(TPASS, "msgrcv(MSG_COPY) got MSGTYPE2 msg data"
>> +			" correctly");
>> +	else
>> +		tst_res(TFAIL, "msgrcv(MSG_COPY) got MSGTYPE2 msg data"
>> +			" incorrectly");
> 
> Can we please keep the strings on a single line?
> 
> I guess that we can shorten the messages a bit e.g.
> 
> tst_res(TFAIL, "msgrcv(MSG_COPY) got MSGTYPE2 data correctly");
> 
> Or even:
> 
> tst_res(TFAIL, "MSG_COPY got MSGTYPE2 data correctly");
> 
> Please try to keep the messages short and to the point.
Will fix it.
> 
>> +	SAFE_MSGCTL(queue_id, IPC_STAT, &buf);
>> +	if (buf.msg_qnum == 2)
>> +		tst_res(TPASS, "msgrcv(MSG_COPY) succeeded, msg queue "
>> +			"still has 2 msg");
>> +	else
>> +		tst_res(TFAIL, "msgrcv(MSG_COPY) msg queue expected 2 msg num,"
>> +			" but only got %d", (int)buf.msg_qnum);
> 
> Here as well we can shorten it to something as:
> 
> 	tst_res(TPASS, "Two messages still in queue");
> 
> 	tst_res(TFAIL, "Expected 2 msgs in queue got %i",
> 		(int)buf.msg_qnum);
> 
Will fix it.
>> +	SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
>> +}
>> +
>>   static void test_zero_msgtyp(void)
>>   {
>>   	prepare_queue();
>> @@ -159,11 +203,28 @@ static void test_negative_msgtyp(void)
>>   static void setup(void)
>>   {
>>   	msgkey = GETIPCKEY();
>> +	prepare_queue();
>> +	TEST(msgrcv(queue_id, &rcv_buf, MSGSIZE, MSGTYPE1, MSG_COPY));
>> +	if (TST_RET != -1)
>> +		tst_res(TINFO, "msgrcv succeeded unexpectedly, kernel doesn't"
>> +			" support MSG_COPY flag");
>> +
>> +	if (TST_ERR == EINVAL) {
>> +		tst_res(TINFO, "msgrcv failed as expected when not using"
>> +			" MSG_COPY and IPC_NOWAIT concurrently");
>> +		msg_copy_sup = 1;
>> +	} else if (TST_ERR == ENOSYS) {
>> +		tst_res(TINFO, "kernel doesn't enable CONFIG_CHECKPOINT_RESTORE");
>> +	} else {
>> +		tst_res(TINFO | TTERRNO, "msgrcv failed when not using MSG_COPY"
>> +			"and IPC_NOWAIT concurrently, expected EINVAL but got");
>> +	}
> 
> This check actually does not work, it fails to detect the support on
> newer kernels, e.g. 5.6.1 without the CONFIG_CHECKPOINT_RESTORE.
> 
> When kernel is new enough call to msgrcv() with MSG_COPY without
> IPC_NOWAIT returns EINVAL even without CONFIG_CHECKPOINT_RESTORE I guess
> that the flags are checked in the generic code and not ifdefed out. You
> will get ENOSYS only on correct combination i.e. MSG_COPY | IPC_NOWAIT.\
Yes, I understand your meaning.
> 
> So why don't we rather:
> 
> * Check if the kernel is older than 3.8 and skip the MSG_COPY test when
>    it is
> 
> * Check for ENOSYS in the test_msg_copy() function
> 
> These two conditions should cover all possible cases.
Yes. msgrcv03 uses this way. Will fix it on v3.
Thanks for your explanation.

Best Regards
Yang Xu
> 

[LTP] [PATCH v3 1/2] syscalls/msgrcv07: Add functional test for MSG_COPY flag

[Yang Xu]

When specifying MSG_COPY flag, we can read the msg but don't destroy
it in msg queue and mtype is interpreted as number of the message to
copy. We check the read data whether correctly and use msgctl to
check whether we still have 2 msg in msg queue after msgrcv(MSG_COPY).

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
v2-v3:
1.modify the detection way about MSG_COPY flag
2.make messages short and to the point
3.zero rcv_buf when call msgrcv the second time

 .../kernel/syscalls/ipc/msgrcv/msgrcv07.c     | 74 +++++++++++++++++--
 1 file changed, 69 insertions(+), 5 deletions(-)

diff --git a/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c b/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c
index 8005a9acd..d4568956f 100644
--- a/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c
+++ b/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c
@@ -2,9 +2,10 @@
 /*
  * Copyright (c) 2014-2020 Fujitsu Ltd.
  * Author: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
+ * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
  *
- * Basic test for msgrcv(2) using MSG_EXCEPT, MSG_NOERROR and different
- * msg_typ(zero,positive,negative).
+ * Basic test for msgrcv(2) using MSG_EXCEPT, MSG_NOERROR, MSG_COPY and
+ * different msg_typ(zero,positive,negative).
  *
  * * With MSG_EXCEPT flag any message type but the one passed to the function
  *   is received.
@@ -12,6 +13,9 @@
  * * With MSG_NOERROR and buffer size less than message size only part of the
  *   buffer is received.
  *
+ * * With MSG_COPY and IPC_NOWAIT flag read the msg but don't destroy it in
+ *   msg queue.
+ *
  * * With msgtyp is 0, then the first message in the queue is read.
  *
  * * With msgtyp is greater than 0, then the first message in the queue of type
@@ -33,7 +37,7 @@
 #define MSG2	"messagetype2"
 
 static key_t msgkey;
-static int queue_id = -1;
+static int queue_id = -1, msg_copy_sup;
 static struct buf {
 	long type;
 	char mtext[MSGSIZE];
@@ -97,6 +101,60 @@ static void test_msg_noerror(void)
 	SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
 }
 
+static void test_msg_copy(void)
+{
+	struct msqid_ds buf = {0};
+
+	if (!msg_copy_sup) {
+		tst_res(TCONF, "MSG_COPY was not supported, skip it");
+		return;
+	}
+	prepare_queue();
+
+	/*
+	 * If MSG_COPY flag was specified, then mtype is interpreted as number
+	 * of the message to copy.
+	 */
+	TEST(msgrcv(queue_id, &rcv_buf, MSGSIZE, 0, MSG_COPY | IPC_NOWAIT));
+	if (TST_RET == -1) {
+		if (TST_ERR == ENOSYS) {
+			tst_res(TCONF,
+				"MSG_COPY needs CONFIG_CHECKPORINT_RESTORE");
+			msg_copy_sup = 0;
+		} else {
+			tst_res(TFAIL | TTERRNO, "msgrcv(0, MSG_COPY) failed");
+		}
+		cleanup();
+		return;
+	}
+	tst_res(TPASS, "msgrcv(0, MSG_COPY) succeeded");
+	if (strcmp(rcv_buf.mtext, MSG1) == 0 && rcv_buf.type == MSGTYPE1)
+		tst_res(TPASS, "MSG_COPY got MSGTYPE1 data correctly");
+	else
+		tst_res(TFAIL, "MSG_COPY got MSGTYPE1 data incorrectly");
+
+	memset(&rcv_buf, 0, sizeof(rcv_buf));
+	TEST(msgrcv(queue_id, &rcv_buf, MSGSIZE, 1, MSG_COPY | IPC_NOWAIT));
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TTERRNO, "msgrcv(1, MSG_COPY) failed");
+		cleanup();
+		return;
+	}
+	tst_res(TPASS, "msgrcv(1, MSG_COPY) succeeded");
+	if (strcmp(rcv_buf.mtext, MSG2) == 0 && rcv_buf.type == MSGTYPE2)
+		tst_res(TPASS, "MSG_COPY got MSGTYPE2 data correctly");
+	else
+		tst_res(TFAIL, "MSG_COPY got MSGTYPE2 data incorrectly");
+
+	SAFE_MSGCTL(queue_id, IPC_STAT, &buf);
+	if (buf.msg_qnum == 2)
+		tst_res(TPASS, "Two messages still in queue");
+	else
+		tst_res(TFAIL, "Expected 2 msgs in queue got %d",
+		       (int)buf.msg_qnum);
+	SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
+}
+
 static void test_zero_msgtyp(void)
 {
 	prepare_queue();
@@ -159,11 +217,17 @@ static void test_negative_msgtyp(void)
 static void setup(void)
 {
 	msgkey = GETIPCKEY();
+
+	if (tst_kvercmp(3, 8, 0) < 0) {
+		tst_res(TCONF, "MSG_COPY needs Linux 3.8 or newer");
+		return;
+	}
+	msg_copy_sup = 1;
 }
 
 static void (*testfunc[])(void) = {test_msg_except, test_msg_noerror,
-				   test_zero_msgtyp, test_positive_msgtyp,
-				   test_negative_msgtyp};
+				   test_msg_copy, test_zero_msgtyp,
+				   test_positive_msgtyp, test_negative_msgtyp};
 
 static void verify_msgcrv(unsigned int n)
 {
-- 
2.23.0

[LTP] [PATCH v3 2/2] syscalls/msgrcv07: improve and short message

[Yang Xu]

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 .../kernel/syscalls/ipc/msgrcv/msgrcv07.c     | 25 +++++++++----------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c b/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c
index d4568956f..04257d257 100644
--- a/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c
+++ b/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c
@@ -72,9 +72,9 @@ static void test_msg_except(void)
 	}
 	tst_res(TPASS, "msgrcv(MSG_EXCEPT) succeeded");
 	if (strcmp(rcv_buf.mtext, MSG1) == 0 && rcv_buf.type == MSGTYPE1)
-		tst_res(TPASS, "msgrcv(MSG_EXCEPT) excepted MSGTYPE2 and only got MSGTYPE1");
+		tst_res(TPASS, "MSG_EXCEPT excepted MSGTYPE2 and got MSGTYPE1");
 	else
-		tst_res(TFAIL, "msgrcv(MSG_EXCEPT) didn't get MSGTYPE1 message");
+		tst_res(TFAIL, "MSG_EXCEPT didn't get MSGTYPE1 message");
 	SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
 }
 
@@ -95,9 +95,9 @@ static void test_msg_noerror(void)
 	}
 	tst_res(TPASS, "msgrcv(MSG_NOERROR) succeeded");
 	if (strncmp(rcv_buf.mtext, MSG1, msg_len) == 0 && rcv_buf.type == MSGTYPE1)
-		tst_res(TPASS, "msgrcv(MSG_NOERROR) truncated message text correctly");
+		tst_res(TPASS, "MSG_NOERROR truncated message correctly");
 	else
-		tst_res(TFAIL, "msgrcv(MSG_NOERROR) truncated message text incorrectly");
+		tst_res(TFAIL, "MSG_NOERROR truncated message incorrectly");
 	SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
 }
 
@@ -167,9 +167,9 @@ static void test_zero_msgtyp(void)
 	}
 	tst_res(TPASS, "msgrcv(zero_msgtyp) succeeded");
 	if (strcmp(rcv_buf.mtext, MSG1) == 0 && rcv_buf.type == MSGTYPE1)
-		tst_res(TPASS, "msgrcv(zero_msgtyp) got the first message");
+		tst_res(TPASS, "zero_msgtyp got the first message");
 	else
-		tst_res(TFAIL, "msgrcv(zero_msgtyp) didn't get the first message");
+		tst_res(TFAIL, "zero_msgtyp didn't get the first message");
 	SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
 }
 
@@ -185,11 +185,11 @@ static void test_positive_msgtyp(void)
 	}
 	tst_res(TPASS, "msgrcv(positive_msgtyp) succeeded");
 	if (strcmp(rcv_buf.mtext, MSG2) == 0 && rcv_buf.type == MSGTYPE2)
-		tst_res(TPASS, "msgrcv(positive_msgtyp) got the first message"
-			       " in the queue of type msgtyp");
+		tst_res(TPASS, "positive_msgtyp got the first message "
+				"in the queue of type msgtyp");
 	else
-		tst_res(TFAIL, "msgrcv(positive_msgtyp) didn't get the first "
-			       "message in the queue of type msgtyp");
+		tst_res(TFAIL, "positive_msgtyp didn't get the first "
+				"message in the queue of type msgtyp");
 	SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
 }
 
@@ -205,15 +205,14 @@ static void test_negative_msgtyp(void)
 	}
 	tst_res(TPASS, "msgrcv(negative_msgtyp) succeeded");
 	if (strcmp(rcv_buf.mtext, MSG1) == 0 && rcv_buf.type == MSGTYPE1)
-		tst_res(TPASS, "msgrcv(negative_msgtyp) got the first message"
+		tst_res(TPASS, "negative_msgtyp got the first message"
 				" in the queue with the lowest type");
 	else
-		tst_res(TFAIL, "msgrcv(negative_msgtyp) didn't get the first "
+		tst_res(TFAIL, "negative_msgtyp didn't get the first "
 				"message in the queue with the lowest type");
 	SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
 }
 
-
 static void setup(void)
 {
 	msgkey = GETIPCKEY();
-- 
2.23.0

[LTP] [PATCH v3 2/2] syscalls/msgrcv07: improve and short message

[Cyril Hrubis]

Hi!
I've added a few more fixes to this patch and pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3 1/2] syscalls/msgrcv07: Add functional test for MSG_COPY flag

[Cyril Hrubis]

Hi!
Pushed with a minor cleanup, thanks.

The diff:

diff --git a/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c b/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c
index d4568956f..82b119d39 100644
--- a/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c
+++ b/testcases/kernel/syscalls/ipc/msgrcv/msgrcv07.c
@@ -106,9 +106,10 @@ static void test_msg_copy(void)
 	struct msqid_ds buf = {0};
 
 	if (!msg_copy_sup) {
-		tst_res(TCONF, "MSG_COPY was not supported, skip it");
+		tst_res(TCONF, "MSG_COPY not supported");
 		return;
 	}
+
 	prepare_queue();
 
 	/*
@@ -124,10 +125,11 @@ static void test_msg_copy(void)
 		} else {
 			tst_res(TFAIL | TTERRNO, "msgrcv(0, MSG_COPY) failed");
 		}
-		cleanup();
-		return;
+		goto exit;
 	}
+
 	tst_res(TPASS, "msgrcv(0, MSG_COPY) succeeded");
+
 	if (strcmp(rcv_buf.mtext, MSG1) == 0 && rcv_buf.type == MSGTYPE1)
 		tst_res(TPASS, "MSG_COPY got MSGTYPE1 data correctly");
 	else
@@ -137,21 +139,25 @@ static void test_msg_copy(void)
 	TEST(msgrcv(queue_id, &rcv_buf, MSGSIZE, 1, MSG_COPY | IPC_NOWAIT));
 	if (TST_RET == -1) {
 		tst_res(TFAIL | TTERRNO, "msgrcv(1, MSG_COPY) failed");
-		cleanup();
-		return;
+		goto exit;
 	}
+
 	tst_res(TPASS, "msgrcv(1, MSG_COPY) succeeded");
+
 	if (strcmp(rcv_buf.mtext, MSG2) == 0 && rcv_buf.type == MSGTYPE2)
 		tst_res(TPASS, "MSG_COPY got MSGTYPE2 data correctly");
 	else
 		tst_res(TFAIL, "MSG_COPY got MSGTYPE2 data incorrectly");
 
 	SAFE_MSGCTL(queue_id, IPC_STAT, &buf);
-	if (buf.msg_qnum == 2)
+	if (buf.msg_qnum == 2) {
 		tst_res(TPASS, "Two messages still in queue");
-	else
+	} else {
 		tst_res(TFAIL, "Expected 2 msgs in queue got %d",
 		       (int)buf.msg_qnum);
+	}
+
+exit:
 	SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
 }
 
@@ -218,11 +224,8 @@ static void setup(void)
 {
 	msgkey = GETIPCKEY();
 
-	if (tst_kvercmp(3, 8, 0) < 0) {
-		tst_res(TCONF, "MSG_COPY needs Linux 3.8 or newer");
-		return;
-	}
-	msg_copy_sup = 1;
+	if (tst_kvercmp(3, 8, 0) >= 0)
+		msg_copy_sup = 1;
 }
 
 static void (*testfunc[])(void) = {test_msg_except, test_msg_noerror,

-- 
Cyril Hrubis
chrubis@suse.cz