All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.