diff mbox series

[RFC] syscalls/ipc: Make use of TST_EXP_FAIL macro

Message ID 1624356737-508-1-git-send-email-xuyang2018.jy@fujitsu.com
State Changes Requested
Headers show
Series [RFC] syscalls/ipc: Make use of TST_EXP_FAIL macro | expand

Commit Message

Yang Xu \(Fujitsu\) June 22, 2021, 10:12 a.m. UTC 
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
Since TST_EXP_FAIL macro only recognizes sycalls succeeded when syscalls return 0,
Can we use this macro directly for the these syscalls's error test? It may result in
invalid retval value and print errno when syscall succeeded. I think it
is a nit and it can improve this api usage range. Is it right?

 testcases/kernel/syscalls/ipc/msgget/msgget02.c | 15 ++-------------
 testcases/kernel/syscalls/ipc/msgget/msgget03.c | 12 ++----------
 testcases/kernel/syscalls/ipc/msgrcv/msgrcv02.c | 14 ++------------
 testcases/kernel/syscalls/ipc/msgrcv/msgrcv03.c | 17 ++---------------
 testcases/kernel/syscalls/ipc/msgrcv/msgrcv05.c | 12 ++----------
 testcases/kernel/syscalls/ipc/msgrcv/msgrcv06.c | 11 ++---------
 testcases/kernel/syscalls/ipc/msgsnd/msgsnd02.c | 14 ++------------
 testcases/kernel/syscalls/ipc/msgsnd/msgsnd05.c | 14 ++------------
 testcases/kernel/syscalls/ipc/msgsnd/msgsnd06.c | 14 ++------------
 9 files changed, 18 insertions(+), 105 deletions(-)

Comments

Cyril Hrubis June 22, 2021, 10:58 a.m. UTC | #1 
Hi!
> Since TST_EXP_FAIL macro only recognizes sycalls succeeded when syscalls return 0,
> Can we use this macro directly for the these syscalls's error test? It may result in
> invalid retval value and print errno when syscall succeeded. I think it
> is a nit and it can improve this api usage range. Is it right?

I guess that it would be slightly cleaner to add more generic macro that
allows us to pass the condition for succeess and build TST_EXP_FAIL() on
the top of that. Maybe something as:

diff --git a/include/tst_test_macros.h b/include/tst_test_macros.h
index 89dfe5a31..4d41741a4 100644
--- a/include/tst_test_macros.h
+++ b/include/tst_test_macros.h
@@ -120,13 +120,13 @@ extern void *TST_RET_PTR;
                        TST_MSG_(TPASS, " passed", #SCALL, ##__VA_ARGS__);     \
        } while (0)                                                            \

-#define TST_EXP_FAIL(SCALL, ERRNO, ...)                                        \
+#define TST_EXP_FAIL_(PASS_COND, SCALL, ERRNO, ...)                            \
        do {                                                                   \
                TEST(SCALL);                                                   \
                                                                               \
                TST_PASS = 0;                                                  \
                                                                               \
-               if (TST_RET == 0) {                                            \
+               if (PASS_COND) {                                               \
                        TST_MSG_(TFAIL, " succeeded", #SCALL, ##__VA_ARGS__);  \
                        break;                                                 \
                }                                                              \
@@ -150,4 +150,8 @@ extern void *TST_RET_PTR;
                }                                                              \
        } while (0)

+#define TST_EXP_FAIL(SCALL, ERRNO, ...) TST_EXP_FAIL_(TST_RET == 0, SCALL, ERRNO)
+
+#define TST_EXP_FAIL2(SCALL, ERRNO, ...) TST_EXP_FAIL_(TST_RET >= 0, SCALL, ERRNO)
+
 #endif /* TST_TEST_MACROS_H__ */

The only hard thing is to find a good name for TST_EXP_FAIL2(), I'm out
of ideas here...
Yang Xu \(Fujitsu\) June 24, 2021, 4:41 a.m. UTC | #2 
Hi Cyril
> Hi!
>> Since TST_EXP_FAIL macro only recognizes sycalls succeeded when syscalls return 0,
>> Can we use this macro directly for the these syscalls's error test? It may result in
>> invalid retval value and print errno when syscall succeeded. I think it
>> is a nit and it can improve this api usage range. Is it right?
>
> I guess that it would be slightly cleaner to add more generic macro that
> allows us to pass the condition for succeess and build TST_EXP_FAIL() on
> the top of that. Maybe something as:
>
> diff --git a/include/tst_test_macros.h b/include/tst_test_macros.h
> index 89dfe5a31..4d41741a4 100644
> --- a/include/tst_test_macros.h
> +++ b/include/tst_test_macros.h
> @@ -120,13 +120,13 @@ extern void *TST_RET_PTR;
>                          TST_MSG_(TPASS, " passed", #SCALL, ##__VA_ARGS__);     \
>          } while (0)                                                            \
>
> -#define TST_EXP_FAIL(SCALL, ERRNO, ...)                                        \
> +#define TST_EXP_FAIL_(PASS_COND, SCALL, ERRNO, ...)                            \
>          do {                                                                   \
>                  TEST(SCALL);                                                   \
>                                                                                 \
>                  TST_PASS = 0;                                                  \
>                                                                                 \
> -               if (TST_RET == 0) {                                            \
> +               if (PASS_COND) {                                               \
>                          TST_MSG_(TFAIL, " succeeded", #SCALL, ##__VA_ARGS__);  \
>                          break;                                                 \
>                  }                                                              \
> @@ -150,4 +150,8 @@ extern void *TST_RET_PTR;
>                  }                                                              \
>          } while (0)
>
> +#define TST_EXP_FAIL(SCALL, ERRNO, ...) TST_EXP_FAIL_(TST_RET == 0, SCALL, ERRNO)
> +
> +#define TST_EXP_FAIL2(SCALL, ERRNO, ...) TST_EXP_FAIL_(TST_RET>= 0, SCALL, ERRNO)
> +
>   #endif /* TST_TEST_MACROS_H__ */
>
> The only hard thing is to find a good name for TST_EXP_FAIL2(), I'm out
> of ideas here...
I think using TST_EXP_FAIL2 directly is simple and clear.

Best Regards
Yang Xu
>
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/ipc/msgget/msgget02.c b/testcases/kernel/syscalls/ipc/msgget/msgget02.c
index a8fac930b..de139790b 100644
--- a/testcases/kernel/syscalls/ipc/msgget/msgget02.c
+++ b/testcases/kernel/syscalls/ipc/msgget/msgget02.c
@@ -46,19 +46,8 @@  static struct tcase {
 
 static void verify_msgget(struct tcase *tc)
 {
-	TEST(msgget(*tc->key, tc->flags));
-
-	if (TST_RET != -1) {
-		tst_res(TFAIL, "msgget() succeeded unexpectedly");
-		return;
-	}
-
-	if (TST_ERR == tc->exp_err) {
-		tst_res(TPASS | TTERRNO, "msgget() failed as expected");
-	} else {
-		tst_res(TFAIL | TTERRNO, "msgget() failed unexpectedly,"
-			" expected %s", tst_strerrno(tc->exp_err));
-	}
+	TST_EXP_FAIL(msgget(*tc->key, tc->flags), tc->exp_err, "msgget(%i, %i)",
+		*tc->key, tc->flags);
 }
 
 static void do_test(unsigned int n)
diff --git a/testcases/kernel/syscalls/ipc/msgget/msgget03.c b/testcases/kernel/syscalls/ipc/msgget/msgget03.c
index 8fa620855..e1f36b3ae 100644
--- a/testcases/kernel/syscalls/ipc/msgget/msgget03.c
+++ b/testcases/kernel/syscalls/ipc/msgget/msgget03.c
@@ -26,16 +26,8 @@  static key_t msgkey;
 
 static void verify_msgget(void)
 {
-	TEST(msgget(msgkey + maxmsgs, IPC_CREAT | IPC_EXCL));
-	if (TST_RET != -1)
-		tst_res(TFAIL, "msgget() succeeded unexpectedly");
-
-	if (TST_ERR == ENOSPC) {
-		tst_res(TPASS | TTERRNO, "msgget() failed as expected");
-	} else {
-		tst_res(TFAIL | TTERRNO, "msgget() failed unexpectedly,"
-			" expected ENOSPC");
-	}
+	TST_EXP_FAIL(msgget(msgkey + maxmsgs, IPC_CREAT | IPC_EXCL), ENOSPC,
+		"msgget(%i, %i)", msgkey + maxmsgs, IPC_CREAT | IPC_EXCL);
 }
 
 static void setup(void)
diff --git a/testcases/kernel/syscalls/ipc/msgrcv/msgrcv02.c b/testcases/kernel/syscalls/ipc/msgrcv/msgrcv02.c
index 8dd28116a..d8df401e7 100644
--- a/testcases/kernel/syscalls/ipc/msgrcv/msgrcv02.c
+++ b/testcases/kernel/syscalls/ipc/msgrcv/msgrcv02.c
@@ -59,18 +59,8 @@  static struct tcase {
 
 static void verify_msgrcv(struct tcase *tc)
 {
-	TEST(msgrcv(*tc->id, tc->buffer, tc->msgsz, tc->msgtyp, tc->msgflag));
-	if (TST_RET != -1) {
-		tst_res(TFAIL, "smgrcv() succeeded unexpectedly");
-		return;
-	}
-
-	if (TST_ERR == tc->exp_err) {
-		tst_res(TPASS | TTERRNO, "msgrcv() failed as expected");
-	} else {
-		tst_res(TFAIL | TTERRNO, "msgrcv() failed unexpectedly,"
-			" expected %s but got", tst_strerrno(tc->exp_err));
-	}
+	TST_EXP_FAIL(msgrcv(*tc->id, tc->buffer, tc->msgsz, tc->msgtyp, tc->msgflag), tc->exp_err,
+		"msgrcv(%i, %p, %i, %ld, %i)", *tc->id, tc->buffer, tc->msgsz, tc->msgtyp, tc->msgflag);
 }
 
 static void do_test(unsigned int n)
diff --git a/testcases/kernel/syscalls/ipc/msgrcv/msgrcv03.c b/testcases/kernel/syscalls/ipc/msgrcv/msgrcv03.c
index b578e2810..8f4b3fa52 100644
--- a/testcases/kernel/syscalls/ipc/msgrcv/msgrcv03.c
+++ b/testcases/kernel/syscalls/ipc/msgrcv/msgrcv03.c
@@ -59,21 +59,8 @@  static void verify_msgrcv(unsigned int n)
 
 	tst_res(TINFO, "%s", tc->message);
 
-	TEST(msgrcv(queue_id, &rcv_buf, MSGSIZE, tc->msg_type, MSG_COPY | tc->msg_flag));
-	if (TST_RET != -1) {
-		tst_res(TFAIL, "msgrcv() succeeded unexpectedly");
-		SAFE_MSGSND(queue_id, &snd_buf, MSGSIZE, 0);
-		return;
-	}
-
-	if (TST_ERR == tc->exp_err) {
-		tst_res(TPASS | TTERRNO, "msgrcv() failed as expected");
-		return;
-	}
-
-	tst_res(TFAIL | TTERRNO,
-		"msgrcv() failed unexpectedly, expected %s got",
-		tst_strerrno(tc->exp_err));
+	TST_EXP_FAIL(msgrcv(queue_id, &rcv_buf, MSGSIZE, tc->msg_type, MSG_COPY | tc->msg_flag), tc->exp_err,
+		"msgrcv(%i, %p, %i, %i, %i)", queue_id, &rcv_buf, MSGSIZE, tc->msg_type, MSG_COPY | tc->msg_flag);
 }
 
 static void setup(void)
diff --git a/testcases/kernel/syscalls/ipc/msgrcv/msgrcv05.c b/testcases/kernel/syscalls/ipc/msgrcv/msgrcv05.c
index 43581896a..6bc908aa5 100644
--- a/testcases/kernel/syscalls/ipc/msgrcv/msgrcv05.c
+++ b/testcases/kernel/syscalls/ipc/msgrcv/msgrcv05.c
@@ -30,16 +30,8 @@  static void sighandler(int sig)
 
 static void verify_msgrcv(void)
 {
-	TEST(msgrcv(queue_id, &rcv_buf, MSGSIZE, 1, 0));
-
-	if (TST_RET != -1) {
-		tst_res(TFAIL, "msgrcv() succeeded unexpectedly");
-		return;
-	}
-	if (TST_ERR == EINTR)
-		tst_res(TPASS | TTERRNO, "msgrcv() failed as expected");
-	else
-		tst_res(TFAIL | TTERRNO, "msgrcv() failed expected EINTR but got");
+	TST_EXP_FAIL(msgrcv(queue_id, &rcv_buf, MSGSIZE, 1, 0), EINTR,
+		"msgrcv(%i, %p, %d, 1, 0)", queue_id, &rcv_buf, MSGSIZE);
 }
 
 static void do_test(void)
diff --git a/testcases/kernel/syscalls/ipc/msgrcv/msgrcv06.c b/testcases/kernel/syscalls/ipc/msgrcv/msgrcv06.c
index 283b4af1d..7fec0b6ad 100644
--- a/testcases/kernel/syscalls/ipc/msgrcv/msgrcv06.c
+++ b/testcases/kernel/syscalls/ipc/msgrcv/msgrcv06.c
@@ -24,15 +24,8 @@  static struct buf {
 
 static void verify_msgrcv(void)
 {
-	TEST(msgrcv(queue_id, &rcv_buf, MSGSIZE, 1, 0));
-	if (TST_RET != -1) {
-		tst_res(TFAIL, "msgrcv() succeeded unexpectedly");
-		return;
-	}
-	if (TST_ERR == EIDRM)
-		tst_res(TPASS | TTERRNO, "msgrcv() failed as expected");
-	else
-		tst_res(TFAIL | TTERRNO, "msgrcv() failed expected EIDRM but got");
+	TST_EXP_FAIL(msgrcv(queue_id, &rcv_buf, MSGSIZE, 1, 0), EIDRM,
+		"msgrcv(%i, %p, %d, 1, 0)", queue_id, &rcv_buf, MSGSIZE);
 }
 
 static void do_test(void)
diff --git a/testcases/kernel/syscalls/ipc/msgsnd/msgsnd02.c b/testcases/kernel/syscalls/ipc/msgsnd/msgsnd02.c
index 24ef6c562..eca660605 100644
--- a/testcases/kernel/syscalls/ipc/msgsnd/msgsnd02.c
+++ b/testcases/kernel/syscalls/ipc/msgsnd/msgsnd02.c
@@ -61,18 +61,8 @@  static struct tcase {
 
 static void verify_msgsnd(struct tcase *tc)
 {
-	TEST(msgsnd(*tc->id, tc->buffer, tc->msgsz, 0));
-	if (TST_RET != -1) {
-		tst_res(TFAIL, "smgsnd() succeeded unexpectedly");
-		return;
-	}
-
-	if (TST_ERR == tc->exp_err) {
-		tst_res(TPASS | TTERRNO, "msgsnd() failed as expected");
-	} else {
-		tst_res(TFAIL | TTERRNO, "msgsnd() failed unexpectedly,"
-			" expected %s", tst_strerrno(tc->exp_err));
-	}
+	TST_EXP_FAIL(msgsnd(*tc->id, tc->buffer, tc->msgsz, 0), tc->exp_err,
+		"msgsnd(%i, %p, %i, 0)", *tc->id, tc->buffer, tc->msgsz);
 }
 
 static void do_test(unsigned int n)
diff --git a/testcases/kernel/syscalls/ipc/msgsnd/msgsnd05.c b/testcases/kernel/syscalls/ipc/msgsnd/msgsnd05.c
index ace32cdaa..f048fa698 100644
--- a/testcases/kernel/syscalls/ipc/msgsnd/msgsnd05.c
+++ b/testcases/kernel/syscalls/ipc/msgsnd/msgsnd05.c
@@ -41,18 +41,8 @@  static struct tcase {
 
 static void verify_msgsnd(struct tcase *tc)
 {
-	TEST(msgsnd(queue_id, &snd_buf, MSGSIZE, tc->flag));
-	if (TST_RET != -1) {
-		tst_res(TFAIL, "msgsnd() succeeded unexpectedly");
-		return;
-	}
-
-	if (TST_ERR == tc->exp_err) {
-		tst_res(TPASS | TTERRNO, "msgsnd() failed as expected");
-	} else {
-		tst_res(TFAIL | TTERRNO, "msgsnd() failed unexpectedly,"
-			" expected %s", tst_strerrno(tc->exp_err));
-	}
+	TST_EXP_FAIL(msgsnd(queue_id, &snd_buf, MSGSIZE, tc->flag), tc->exp_err,
+		"msgsnd(%i, %p, %i, %i)", queue_id, &snd_buf, MSGSIZE, tc->flag);
 }
 
 static void sighandler(int sig)
diff --git a/testcases/kernel/syscalls/ipc/msgsnd/msgsnd06.c b/testcases/kernel/syscalls/ipc/msgsnd/msgsnd06.c
index 9f462b672..8fc665e68 100644
--- a/testcases/kernel/syscalls/ipc/msgsnd/msgsnd06.c
+++ b/testcases/kernel/syscalls/ipc/msgsnd/msgsnd06.c
@@ -28,18 +28,8 @@  static struct buf {
 
 static void verify_msgsnd(void)
 {
-	TEST(msgsnd(queue_id, &snd_buf, MSGSIZE, 0));
-	if (TST_RET != -1) {
-		tst_res(TFAIL, "msgsnd() succeeded unexpectedly");
-		return;
-	}
-
-	if (TST_ERR == EIDRM) {
-		tst_res(TPASS | TTERRNO, "msgsnd() failed as expected");
-	} else {
-		tst_res(TFAIL | TTERRNO,
-			"msgsnd() failed unexpectedly, expected EIDRM");
-	}
+	TST_EXP_FAIL(msgsnd(queue_id, &snd_buf, MSGSIZE, 0), EIDRM,
+		"msgsnd(%i, %p, %i, 0)", queue_id, &snd_buf, MSGSIZE);
 }
 
 static void do_test(void)