public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
* [LTP] [PATCH 0/3] TST_EXP_*() macros fixes
@ 2021-08-23 15:05 Cyril Hrubis
  2021-08-23 15:05 ` [LTP] [PATCH 1/3] tst_test_macros: Fix TST_EXP_*() default message Cyril Hrubis
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Cyril Hrubis @ 2021-08-23 15:05 UTC (permalink / raw)
  To: ltp

Cyril Hrubis (3):
  tst_test_macros: Fix TST_EXP_*() default message
  tst_test_macros: Stringify early
  tst_test_macros: Add test_macros05 tests

 include/tst_test_macros.h        | 40 ++++++++++++++-------------
 lib/newlib_tests/.gitignore      |  1 +
 lib/newlib_tests/test_macros05.c | 46 ++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 19 deletions(-)
 create mode 100644 lib/newlib_tests/test_macros05.c

-- 
2.31.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [LTP] [PATCH 1/3] tst_test_macros: Fix TST_EXP_*() default message
  2021-08-23 15:05 [LTP] [PATCH 0/3] TST_EXP_*() macros fixes Cyril Hrubis
@ 2021-08-23 15:05 ` Cyril Hrubis
  2021-08-24  7:35   ` Li Wang
  2021-08-23 15:05 ` [LTP] [PATCH 2/3] tst_test_macros: Stringify early Cyril Hrubis
  2021-08-23 15:05 ` [LTP] [PATCH 3/3] tst_test_macros: Add test_macros05 tests Cyril Hrubis
  2 siblings, 1 reply; 10+ messages in thread
From: Cyril Hrubis @ 2021-08-23 15:05 UTC (permalink / raw)
  To: ltp

We have to erase the last comma because otherwise we pass down one more
empty parameter to the TST_EXP_*_() macros which then causes the FMT
string to be empty and we end up with an empty default message.

Consider for example:

TST_EXP_FD(open(fname, O_RDONLY));

Before the patch it would produce:

foo.c:12: TPASS: returned fd 4

After it would produce:

foo.c:12: TPASS: open(fname, O_RDONLY) returned fd 4

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 include/tst_test_macros.h | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/tst_test_macros.h b/include/tst_test_macros.h
index d9d7f930f..ecc612b0d 100644
--- a/include/tst_test_macros.h
+++ b/include/tst_test_macros.h
@@ -70,7 +70,7 @@ extern void *TST_RET_PTR;
 
 #define TST_EXP_POSITIVE(SCALL, ...)                               \
 	do {                                                       \
-		TST_EXP_POSITIVE_(SCALL, __VA_ARGS__);             \
+		TST_EXP_POSITIVE_(SCALL, ##__VA_ARGS__);           \
 		                                                   \
 		if (TST_PASS) {                                    \
 			TST_MSGP_(TPASS, " returned %ld",          \
@@ -78,22 +78,22 @@ extern void *TST_RET_PTR;
 		}                                                  \
 	} while (0)
 
-#define TST_EXP_FD_SILENT(SCALL, ...)	TST_EXP_POSITIVE_(SCALL, __VA_ARGS__)
+#define TST_EXP_FD_SILENT(SCALL, ...)	TST_EXP_POSITIVE_(SCALL, ##__VA_ARGS__)
 
 #define TST_EXP_FD(SCALL, ...)                                                 \
 	do {                                                                   \
-		TST_EXP_FD_SILENT(SCALL, __VA_ARGS__);                         \
+		TST_EXP_FD_SILENT(SCALL, ##__VA_ARGS__);                       \
 		                                                               \
 		if (TST_PASS)                                                  \
 			TST_MSGP_(TPASS, " returned fd %ld", TST_RET,          \
 				#SCALL, ##__VA_ARGS__);                        \
 	} while (0)
 
-#define TST_EXP_PID_SILENT(SCALL, ...)	TST_EXP_POSITIVE_(SCALL, __VA_ARGS__)
+#define TST_EXP_PID_SILENT(SCALL, ...)	TST_EXP_POSITIVE_(SCALL, ##__VA_ARGS__)
 
 #define TST_EXP_PID(SCALL, ...)                                                \
 	do {                                                                   \
-		TST_EXP_PID_SILENT(SCALL, __VA_ARGS__);                        \
+		TST_EXP_PID_SILENT(SCALL, ##__VA_ARGS__);                      \
 									       \
 		if (TST_PASS)                                                  \
 			TST_MSGP_(TPASS, " returned pid %ld", TST_RET,         \
@@ -124,7 +124,7 @@ extern void *TST_RET_PTR;
 
 #define TST_EXP_PASS(SCALL, ...)                                               \
 	do {                                                                   \
-		TST_EXP_PASS_SILENT(SCALL, __VA_ARGS__);                       \
+		TST_EXP_PASS_SILENT(SCALL, ##__VA_ARGS__);                     \
 		                                                               \
 		if (TST_PASS)                                                  \
 			TST_MSG_(TPASS, " passed", #SCALL, ##__VA_ARGS__);     \
@@ -158,8 +158,8 @@ extern void *TST_RET_PTR;
 		}							\
 	} while (0)
 
-#define TST_EXP_FAIL(SCALL, ERRNO, ...) TST_EXP_FAIL_(TST_RET == 0, SCALL, ERRNO, __VA_ARGS__)
+#define TST_EXP_FAIL(SCALL, ERRNO, ...) TST_EXP_FAIL_(TST_RET == 0, SCALL, ERRNO, ##__VA_ARGS__)
 
-#define TST_EXP_FAIL2(SCALL, ERRNO, ...) TST_EXP_FAIL_(TST_RET >= 0, SCALL, ERRNO, __VA_ARGS__)
+#define TST_EXP_FAIL2(SCALL, ERRNO, ...) TST_EXP_FAIL_(TST_RET >= 0, SCALL, ERRNO, ##__VA_ARGS__)
 
 #endif	/* TST_TEST_MACROS_H__ */
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [LTP] [PATCH 2/3] tst_test_macros: Stringify early
  2021-08-23 15:05 [LTP] [PATCH 0/3] TST_EXP_*() macros fixes Cyril Hrubis
  2021-08-23 15:05 ` [LTP] [PATCH 1/3] tst_test_macros: Fix TST_EXP_*() default message Cyril Hrubis
@ 2021-08-23 15:05 ` Cyril Hrubis
  2021-08-23 15:05 ` [LTP] [PATCH 3/3] tst_test_macros: Add test_macros05 tests Cyril Hrubis
  2 siblings, 0 replies; 10+ messages in thread
From: Cyril Hrubis @ 2021-08-23 15:05 UTC (permalink / raw)
  To: ltp

We have to stringify the SCALL argument early, otherwise it will be
replaced in a case that it's a macro.

This is not fixing any hypotetical problem for instance the
tst_syscall() is a macro. If we pass it to a TST_EXP_*() macro and keep
it as a default message it will produce messy output.

Take for an example:

TST_EXP_FAIL(tst_sycall(__NR_foo, 1), EINVAL);

Before this patch it produces:

foo.c:12: TPASS: ({ int tst_ret; if (277 == -1) { (*__errno_location ()) = 38; tst_ret = -1; } else { tst_ret = syscall(277, 1); } if (tst_ret == -1 && (*__errno_location ()) == 38) { ({ do { ((void)sizeof(char[1 - 2 * !!(!((32) & (2 | 32 | 1)))])); } while (0); tst_brk_("foo.c", 12, (32), ("syscall(-1496763488) " "__NR_foo" " not supported"),277); }); } tst_ret; }) : EINVAL (22)

After this patch it will produce:

foo.c:12: TPASS: tst_syscall(__NR_foo, 1) : EINVAL (22)

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 include/tst_test_macros.h | 40 ++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/include/tst_test_macros.h b/include/tst_test_macros.h
index ecc612b0d..50598aa15 100644
--- a/include/tst_test_macros.h
+++ b/include/tst_test_macros.h
@@ -46,7 +46,7 @@ extern void *TST_RET_PTR;
 	tst_res_(__FILE__, __LINE__, RES, \
 		TST_FMT_(TST_2_(dummy, ##__VA_ARGS__, SCALL) FMT, __VA_ARGS__), PAR)
 
-#define TST_EXP_POSITIVE_(SCALL, ...)                                          \
+#define TST_EXP_POSITIVE_(SCALL, SSCALL, ...)                                  \
 	do {                                                                   \
 		TEST(SCALL);                                                   \
 		                                                               \
@@ -54,13 +54,13 @@ extern void *TST_RET_PTR;
 		                                                               \
 		if (TST_RET == -1) {                                           \
 			TST_MSG_(TFAIL | TTERRNO, " failed",                   \
-			         #SCALL, ##__VA_ARGS__);                       \
+			         SSCALL, ##__VA_ARGS__);                       \
 			break;                                                 \
 		}                                                              \
 		                                                               \
 		if (TST_RET < 0) {                                             \
 			TST_MSGP_(TFAIL | TTERRNO, " invalid retval %ld",      \
-			          TST_RET, #SCALL, ##__VA_ARGS__);             \
+			          TST_RET, SSCALL, ##__VA_ARGS__);             \
 			break;                                                 \
 		}                                                              \
                                                                                \
@@ -70,7 +70,7 @@ extern void *TST_RET_PTR;
 
 #define TST_EXP_POSITIVE(SCALL, ...)                               \
 	do {                                                       \
-		TST_EXP_POSITIVE_(SCALL, ##__VA_ARGS__);           \
+		TST_EXP_POSITIVE_(SCALL, #SCALL, ##__VA_ARGS__);   \
 		                                                   \
 		if (TST_PASS) {                                    \
 			TST_MSGP_(TPASS, " returned %ld",          \
@@ -78,29 +78,29 @@ extern void *TST_RET_PTR;
 		}                                                  \
 	} while (0)
 
-#define TST_EXP_FD_SILENT(SCALL, ...)	TST_EXP_POSITIVE_(SCALL, ##__VA_ARGS__)
+#define TST_EXP_FD_SILENT(SCALL, ...)	TST_EXP_POSITIVE_(SCALL, #SCALL, ##__VA_ARGS__)
 
 #define TST_EXP_FD(SCALL, ...)                                                 \
 	do {                                                                   \
-		TST_EXP_FD_SILENT(SCALL, ##__VA_ARGS__);                       \
+		TST_EXP_POSITIVE_(SCALL, #SCALL, ##__VA_ARGS__);               \
 		                                                               \
 		if (TST_PASS)                                                  \
 			TST_MSGP_(TPASS, " returned fd %ld", TST_RET,          \
 				#SCALL, ##__VA_ARGS__);                        \
 	} while (0)
 
-#define TST_EXP_PID_SILENT(SCALL, ...)	TST_EXP_POSITIVE_(SCALL, ##__VA_ARGS__)
+#define TST_EXP_PID_SILENT(SCALL, ...)	TST_EXP_POSITIVE_(SCALL, #SCALL, ##__VA_ARGS__)
 
 #define TST_EXP_PID(SCALL, ...)                                                \
 	do {                                                                   \
-		TST_EXP_PID_SILENT(SCALL, ##__VA_ARGS__);                      \
+		TST_EXP_POSITIVE_(SCALL, #SCALL, ##__VA_ARGS__);               \
 									       \
 		if (TST_PASS)                                                  \
 			TST_MSGP_(TPASS, " returned pid %ld", TST_RET,         \
 				#SCALL, ##__VA_ARGS__);                        \
 	} while (0)
 
-#define TST_EXP_PASS_SILENT(SCALL, ...)                                        \
+#define TST_EXP_PASS_SILENT_(SCALL, SSCALL, ...)                               \
 	do {                                                                   \
 		TEST(SCALL);                                                   \
 		                                                               \
@@ -108,13 +108,13 @@ extern void *TST_RET_PTR;
 		                                                               \
 		if (TST_RET == -1) {                                           \
 			TST_MSG_(TFAIL | TTERRNO, " failed",                   \
-			         #SCALL, ##__VA_ARGS__);                       \
+			         SSCALL, ##__VA_ARGS__);                       \
 		        break;                                                 \
 		}                                                              \
 		                                                               \
 		if (TST_RET != 0) {                                            \
 			TST_MSGP_(TFAIL | TTERRNO, " invalid retval %ld",      \
-			          TST_RET, #SCALL, ##__VA_ARGS__);             \
+			          TST_RET, SSCALL, ##__VA_ARGS__);             \
 			break;                                                 \
 		}                                                              \
                                                                                \
@@ -122,44 +122,46 @@ extern void *TST_RET_PTR;
                                                                                \
 	} while (0)
 
+#define TST_EXP_PASS_SILENT(SCALL, ...) TST_EXP_PASS_SILENT_(SCALL, #SCALL, ##__VA_ARGS__)
+
 #define TST_EXP_PASS(SCALL, ...)                                               \
 	do {                                                                   \
-		TST_EXP_PASS_SILENT(SCALL, ##__VA_ARGS__);                     \
+		TST_EXP_PASS_SILENT_(SCALL, #SCALL, ##__VA_ARGS__);            \
 		                                                               \
 		if (TST_PASS)                                                  \
 			TST_MSG_(TPASS, " passed", #SCALL, ##__VA_ARGS__);     \
 	} while (0)                                                            \
 
-#define TST_EXP_FAIL_(PASS_COND, SCALL, ERRNO, ...)                            \
+#define TST_EXP_FAIL_(PASS_COND, SCALL, SSCALL, ERRNO, ...)                    \
 	do {                                                                   \
 		TEST(SCALL);                                                   \
 		                                                               \
 		TST_PASS = 0;                                                  \
 		                                                               \
 		if (PASS_COND) {                                               \
-			TST_MSG_(TFAIL, " succeeded", #SCALL, ##__VA_ARGS__);  \
+			TST_MSG_(TFAIL, " succeeded", SSCALL, ##__VA_ARGS__);  \
 		        break;                                                 \
 		}                                                              \
 		                                                               \
 		if (TST_RET != -1) {                                           \
 			TST_MSGP_(TFAIL | TTERRNO, " invalid retval %ld",      \
-			          TST_RET, #SCALL, ##__VA_ARGS__);             \
+			          TST_RET, SSCALL, ##__VA_ARGS__);             \
 			break;                                                 \
 		}                                                              \
 		                                                               \
 		if (TST_ERR == ERRNO) {					\
 			TST_MSG_(TPASS | TTERRNO, " ",			\
-				 #SCALL, ##__VA_ARGS__);		\
+				 SSCALL, ##__VA_ARGS__);		\
 			TST_PASS = 1;					\
 		} else {						\
 			TST_MSGP_(TFAIL | TTERRNO, " expected %s",	\
 				  tst_strerrno(ERRNO),			\
-				  #SCALL, ##__VA_ARGS__);		\
+				  SSCALL, ##__VA_ARGS__);		\
 		}							\
 	} while (0)
 
-#define TST_EXP_FAIL(SCALL, ERRNO, ...) TST_EXP_FAIL_(TST_RET == 0, SCALL, ERRNO, ##__VA_ARGS__)
+#define TST_EXP_FAIL(SCALL, ERRNO, ...) TST_EXP_FAIL_(TST_RET == 0, SCALL, #SCALL, ERRNO, ##__VA_ARGS__)
 
-#define TST_EXP_FAIL2(SCALL, ERRNO, ...) TST_EXP_FAIL_(TST_RET >= 0, SCALL, ERRNO, ##__VA_ARGS__)
+#define TST_EXP_FAIL2(SCALL, ERRNO, ...) TST_EXP_FAIL_(TST_RET >= 0, SCALL, #SCALL, ERRNO, ##__VA_ARGS__)
 
 #endif	/* TST_TEST_MACROS_H__ */
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [LTP] [PATCH 3/3] tst_test_macros: Add test_macros05 tests
  2021-08-23 15:05 [LTP] [PATCH 0/3] TST_EXP_*() macros fixes Cyril Hrubis
  2021-08-23 15:05 ` [LTP] [PATCH 1/3] tst_test_macros: Fix TST_EXP_*() default message Cyril Hrubis
  2021-08-23 15:05 ` [LTP] [PATCH 2/3] tst_test_macros: Stringify early Cyril Hrubis
@ 2021-08-23 15:05 ` Cyril Hrubis
  2 siblings, 0 replies; 10+ messages in thread
From: Cyril Hrubis @ 2021-08-23 15:05 UTC (permalink / raw)
  To: ltp

That tests the two corner cases with default message and macro
stringification.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 lib/newlib_tests/.gitignore      |  1 +
 lib/newlib_tests/test_macros05.c | 46 ++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)
 create mode 100644 lib/newlib_tests/test_macros05.c

diff --git a/lib/newlib_tests/.gitignore b/lib/newlib_tests/.gitignore
index 807e510ee..a0bad78c1 100644
--- a/lib/newlib_tests/.gitignore
+++ b/lib/newlib_tests/.gitignore
@@ -41,6 +41,7 @@ test_macros01
 test_macros02
 test_macros03
 test_macros04
+test_macros05
 tst_fuzzy_sync01
 tst_fuzzy_sync02
 tst_fuzzy_sync03
diff --git a/lib/newlib_tests/test_macros05.c b/lib/newlib_tests/test_macros05.c
new file mode 100644
index 000000000..a164f23ac
--- /dev/null
+++ b/lib/newlib_tests/test_macros05.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2021 Cyril Hrubis <chrubis@suse.cz>
+ */
+
+/*
+ * Tests various corner conditions:
+ *
+ * - default message, i.e. first argument stringification
+ * - macro indirection, i.e. we have to stringify early
+ *
+ * The output should include the MACRO_FAIL() as the either fail of pass
+ * message. If it's missing or if it has been replaced by the function name
+ * there is a bug in the TST_EXP_*() macro.
+ */
+
+#include "tst_test.h"
+
+static int fail_fn_should_not_be_seen_in_output(void)
+{
+	errno = EINVAL;
+	return -1;
+}
+
+#define MACRO_FAIL() fail_fn_should_not_be_seen_in_output()
+
+static void do_test(void)
+{
+	TST_EXP_FAIL(MACRO_FAIL(), EINVAL);
+	TST_EXP_FAIL2(MACRO_FAIL(), EINVAL);
+
+	TST_EXP_PASS(MACRO_FAIL());
+	TST_EXP_PASS_SILENT(MACRO_FAIL());
+
+	TST_EXP_PID(MACRO_FAIL());
+	TST_EXP_PID_SILENT(MACRO_FAIL());
+
+	TST_EXP_FD(MACRO_FAIL());
+	TST_EXP_FD_SILENT(MACRO_FAIL());
+
+	TST_EXP_POSITIVE(MACRO_FAIL());
+}
+
+static struct tst_test test = {
+	.test_all = do_test,
+};
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [LTP] [PATCH 1/3] tst_test_macros: Fix TST_EXP_*() default message
  2021-08-23 15:05 ` [LTP] [PATCH 1/3] tst_test_macros: Fix TST_EXP_*() default message Cyril Hrubis
@ 2021-08-24  7:35   ` Li Wang
  2021-08-24  8:48     ` Cyril Hrubis
  2021-08-24 11:48     ` Petr Vorel
  0 siblings, 2 replies; 10+ messages in thread
From: Li Wang @ 2021-08-24  7:35 UTC (permalink / raw)
  To: ltp

On Mon, Aug 23, 2021 at 11:05 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> We have to erase the last comma because otherwise we pass down one more
> empty parameter to the TST_EXP_*_() macros which then causes the FMT
> string to be empty and we end up with an empty default message.
>


Patchset looks good from the code layer though it is a bit complicate
in 2/3 stringify handling, but that should be acceptable.

My only hesitating is about ##__VA_ARGS__, because it says that is still
as GNU's extension but have not got into standard C.

So I especially run with GitHub CI and got pretty compiling results, then I
have
a positive attitude on applying these patches unless there is someone who
blames it broken something in a general (but I guess the possibility is
very little).
https://github.com/wangli5665/ltp/runs/3408080506

Feel free to add my reviews:
Reviewed-by: Li Wang <liwang@redhat.com>

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210824/294f5d33/attachment.htm>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [LTP] [PATCH 1/3] tst_test_macros: Fix TST_EXP_*() default message
  2021-08-24  7:35   ` Li Wang
@ 2021-08-24  8:48     ` Cyril Hrubis
  2021-08-24  9:29       ` Li Wang
  2021-08-24 11:48     ` Petr Vorel
  1 sibling, 1 reply; 10+ messages in thread
From: Cyril Hrubis @ 2021-08-24  8:48 UTC (permalink / raw)
  To: ltp

Hi!
> Patchset looks good from the code layer though it is a bit complicate
> in 2/3 stringify handling, but that should be acceptable.
> 
> My only hesitating is about ##__VA_ARGS__, because it says that is still
> as GNU's extension but have not got into standard C.

Note that we have been using it in the header from the start. There were
just a few places where it was missing, mostly in the variants that have
been added later.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [LTP] [PATCH 1/3] tst_test_macros: Fix TST_EXP_*() default message
  2021-08-24  8:48     ` Cyril Hrubis
@ 2021-08-24  9:29       ` Li Wang
  2021-08-24 12:46         ` Richard Palethorpe
  0 siblings, 1 reply; 10+ messages in thread
From: Li Wang @ 2021-08-24  9:29 UTC (permalink / raw)
  To: ltp

On Tue, Aug 24, 2021 at 4:48 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> Hi!
> > Patchset looks good from the code layer though it is a bit complicate
> > in 2/3 stringify handling, but that should be acceptable.
> >
> > My only hesitating is about ##__VA_ARGS__, because it says that is still
> > as GNU's extension but have not got into standard C.
>
> Note that we have been using it in the header from the start. There were
> just a few places where it was missing, mostly in the variants that have
> been added later.
>

Ah great, I was neglect that point.  Hence it should be safe enough.

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210824/e05904b8/attachment-0001.htm>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [LTP] [PATCH 1/3] tst_test_macros: Fix TST_EXP_*() default message
  2021-08-24  7:35   ` Li Wang
  2021-08-24  8:48     ` Cyril Hrubis
@ 2021-08-24 11:48     ` Petr Vorel
  2021-08-24 13:37       ` Cyril Hrubis
  1 sibling, 1 reply; 10+ messages in thread
From: Petr Vorel @ 2021-08-24 11:48 UTC (permalink / raw)
  To: ltp

Hi Cyril, Li,

to whole patchset
Reviewed-by: Petr Vorel <pvorel@suse.cz>

> So I especially run with GitHub CI and got pretty compiling results, then I
> have
> a positive attitude on applying these patches unless there is someone who
> blames it broken something in a general (but I guess the possibility is
> very little).
> https://github.com/wangli5665/ltp/runs/3408080506

Thanks! I've also tested it in github to catch things early.

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [LTP] [PATCH 1/3] tst_test_macros: Fix TST_EXP_*() default message
  2021-08-24  9:29       ` Li Wang
@ 2021-08-24 12:46         ` Richard Palethorpe
  0 siblings, 0 replies; 10+ messages in thread
From: Richard Palethorpe @ 2021-08-24 12:46 UTC (permalink / raw)
  To: ltp

Hello,

Li Wang <liwang@redhat.com> writes:

> On Tue, Aug 24, 2021 at 4:48 PM Cyril Hrubis <chrubis@suse.cz> wrote:
>
>> Hi!
>> > Patchset looks good from the code layer though it is a bit complicate
>> > in 2/3 stringify handling, but that should be acceptable.
>> >
>> > My only hesitating is about ##__VA_ARGS__, because it says that is still
>> > as GNU's extension but have not got into standard C.
>>
>> Note that we have been using it in the header from the start. There were
>> just a few places where it was missing, mostly in the variants that have
>> been added later.
>>
>
> Ah great, I was neglect that point.  Hence it should be safe enough.
>
> -- 
> Regards,
> Li Wang

There is also __VA_OPT__(,) which can't be confused with concatenation.

I'm not sure we should use it thought, it seems like it was only
recently added to other compilers. Still might be a good idea to link to
the following page somewhere:

https://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html#Variadic-Macros

LGTM otherwise!

-- 
Thank you,
Richard.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [LTP] [PATCH 1/3] tst_test_macros: Fix TST_EXP_*() default message
  2021-08-24 11:48     ` Petr Vorel
@ 2021-08-24 13:37       ` Cyril Hrubis
  0 siblings, 0 replies; 10+ messages in thread
From: Cyril Hrubis @ 2021-08-24 13:37 UTC (permalink / raw)
  To: ltp

Hi!
Thanks for the reviews, pushed.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-08-24 13:37 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-08-23 15:05 [LTP] [PATCH 0/3] TST_EXP_*() macros fixes Cyril Hrubis
2021-08-23 15:05 ` [LTP] [PATCH 1/3] tst_test_macros: Fix TST_EXP_*() default message Cyril Hrubis
2021-08-24  7:35   ` Li Wang
2021-08-24  8:48     ` Cyril Hrubis
2021-08-24  9:29       ` Li Wang
2021-08-24 12:46         ` Richard Palethorpe
2021-08-24 11:48     ` Petr Vorel
2021-08-24 13:37       ` Cyril Hrubis
2021-08-23 15:05 ` [LTP] [PATCH 2/3] tst_test_macros: Stringify early Cyril Hrubis
2021-08-23 15:05 ` [LTP] [PATCH 3/3] tst_test_macros: Add test_macros05 tests Cyril Hrubis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox