syscalls/mq: don't use TWARN with tst_brk()

PATCH New Jan Stancek 5 participants 22 messages Nov 8, 2018 – Jan 8, 2019

lore

Share & tools

lore

Jan Stancek

author

2018-11-08 20:59 UTC #0 lore raw

PATCH 1/2 syscalls/mq: don't use TWARN with tst_brk()

Signed-off-by Jan Stancek <[email protected]>

---
 testcases/kernel/syscalls/utils/mq.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

testcases/kernel/syscalls/utils/mq.h +1 -1

index 0bdac97229cd..b55228c885dc 100644
@@ -74,7 +74,7 @@ static void cleanup_queue(mqd_t fd)
 
 	memset(&mqstat, 0, sizeof(mqstat));
 	if (mq_getattr(fd, &mqstat) == -1) {
-		tst_brk(TWARN, "mq_getattr() failed");
+		tst_brk(TBROK|TERRNO, "mq_getattr() failed");
 		return;
 	}

Signature · 1 line

1.8.3.1

Jan Stancek

author

2018-11-08 20:59 UTC #1 lore raw

PATCH 2/2 lib: build check parameters for tst_brk()

This patch adds simple build-check that allows only TFAIL, TBROK and TCONF as parameter for tst_brk().

TFAIL is currently quite commonly used as a shortcut for TFAIL + exit() by many tests. I kept it for now, since it doesn't go against current doc description.

Per kernel comments this approach works fine for simple cases, which should be sufficient for LTP, e.g. we don't pass TBROK as a paramter to inlined function, that passes it further down to tst_brk().

Signed-off-by Jan Stancek <[email protected]>

---
 include/tst_common.h | 3 +++
 include/tst_test.h   | 7 +++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

include/tst_common.h +3 -0

index 27924766ef6e..358f2a76ecda 100644
@@ -65,4 +65,7 @@
 	ERET;								\
 })
 
+#define BUILD_BUG_ON(condition) \
+	do { ((void)sizeof(char[1 - 2 * !!(condition)])); } while (0)
+
 #endif /* TST_COMMON_H__ */

include/tst_test.h +5 -2

index 2ebf746eb720..cd936eb792bd 100644
@@ -69,8 +69,11 @@ void tst_brk_(const char *file, const int lineno, int ttype,
               const char *fmt, ...)
               __attribute__ ((format (printf, 4, 5)));
 
-#define tst_brk(ttype, arg_fmt, ...) \
-	tst_brk_(__FILE__, __LINE__, (ttype), (arg_fmt), ##__VA_ARGS__)
+#define tst_brk(ttype, arg_fmt, ...)						\
+	({									\
+		BUILD_BUG_ON(!((ttype) & (TBROK | TCONF | TFAIL)));		\
+		tst_brk_(__FILE__, __LINE__, (ttype), (arg_fmt), ##__VA_ARGS__);\
+	})
 
 /* flush stderr and stdout */
 void tst_flush(void);

Signature · 1 line

1.8.3.1

Xiao Yang

2018-11-09 01:56 UTC #2 lore raw

PATCH 2/2 lib: build check parameters for tst_brk()

Quoted 35 lines · Jan Stancek

This patch adds simple build-check that allows only TFAIL, TBROK and TCONF as parameter for tst_brk().
TFAIL is currently quite commonly used as a shortcut for TFAIL + exit() by many tests. I kept it for now, since it doesn't go against current doc description.
Per kernel comments this approach works fine for simple cases, which should be sufficient for LTP, e.g. we don't pass TBROK as a paramter to inlined function, that passes it further down to tst_brk().
Signed-off-by Jan Stancek<[email protected]>
---
  include/tst_common.h | 3 +++
  include/tst_test.h   | 7 +++++--
  2 files changed, 8 insertions(+), 2 deletions(-)
include/tst_common.h +3 -0
index 27924766ef6e..358f2a76ecda 100644
@@ -65,4 +65,7 @@
  	ERET;								\
  })
 
+#define BUILD_BUG_ON(condition) \
+	do { ((void)sizeof(char[1 - 2 * !!(condition)])); } while (0)
+
  #endif /* TST_COMMON_H__ */
include/tst_test.h +0 -0
index 2ebf746eb720..cd936eb792bd 100644

include/tst_test.h void tst_brk_(const char *file, const int lineno, int ttype, · line 76

Quoted 10 lines

include/tst_test.h +5 -2

@@ -69,8 +69,11 @@ void tst_brk_(const char *file, const int lineno, int ttype,
                const char *fmt, ...)
                __attribute__ ((format (printf, 4, 5)));
 
-#define tst_brk(ttype, arg_fmt, ...) \
-	tst_brk_(__FILE__, __LINE__, (ttype), (arg_fmt), ##__VA_ARGS__)
+#define tst_brk(ttype, arg_fmt, ...)						\
+	({									\
+		BUILD_BUG_ON(!((ttype)&  (TBROK | TCONF | TFAIL)));		\
+		tst_brk_(__FILE__, __LINE__, (ttype), (arg_fmt), ##__VA_ARGS__);\
+	})

Hi Jan,

Perhaps, we could add some hints about the invalid ttype. e.g. "tst_brk(): invalid type, please use TBROK/TCONF/TFAIL"

Other than that, this patch set looks good to me.

Best Regards,
Xiao Yang

Quoted 3 lines

include/tst_test.h +0 -0

  /* flush stderr and stdout */
  void tst_flush(void);

Jan Stancek

author

2018-11-09 17:57 UTC #3 lore raw

PATCH 2/2 lib: build check parameters for tst_brk()

----- Original Message -----

Quoted 49 lines · Jan Stancek

Quoted 36 lines
This patch adds simple build-check that allows only TFAIL, TBROK and TCONF as parameter for tst_brk().
TFAIL is currently quite commonly used as a shortcut for TFAIL + exit() by many tests. I kept it for now, since it doesn't go against current doc description.
Per kernel comments this approach works fine for simple cases, which should be sufficient for LTP, e.g. we don't pass TBROK as a paramter to inlined function, that passes it further down to tst_brk().
Signed-off-by Jan Stancek<[email protected]>
---
  include/tst_common.h | 3 +++
  include/tst_test.h   | 7 +++++--
  2 files changed, 8 insertions(+), 2 deletions(-)
include/tst_common.h +3 -0
index 27924766ef6e..358f2a76ecda 100644
@@ -65,4 +65,7 @@
  	ERET;								\
  })
 
+#define BUILD_BUG_ON(condition) \
+	do { ((void)sizeof(char[1 - 2 * !!(condition)])); } while (0)
+
  #endif /* TST_COMMON_H__ */
include/tst_test.h +0 -0
index 2ebf746eb720..cd936eb792bd 100644
@@ -69,8 +69,11 @@ void tst_brk_(const char *file, const int lineno, int
 ttype,
include/tst_test.h void tst_brk_(const char *file, const int lineno, int · line 77
Quoted 10 lines
include/tst_test.h +5 -2
                const char *fmt, ...)
                __attribute__ ((format (printf, 4, 5)));
 
-#define tst_brk(ttype, arg_fmt, ...) \
-	tst_brk_(__FILE__, __LINE__, (ttype), (arg_fmt), ##__VA_ARGS__)
+#define tst_brk(ttype, arg_fmt, ...)						\
+	({									\
+		BUILD_BUG_ON(!((ttype)&  (TBROK | TCONF | TFAIL)));		\
+		tst_brk_(__FILE__, __LINE__, (ttype), (arg_fmt), ##__VA_ARGS__);\
+	})
Hi Jan,

Quoted 2 lines

Perhaps, we could add some hints about the invalid ttype. e.g. "tst_brk(): invalid type, please use TBROK/TCONF/TFAIL"

Do you have suggestion how to achieve that?

There's an __attribute__(error), but it's supported only from gcc 4.3 as I recall. Alternative would be link time failure, with a symbol name suggesting what went wrong.

Regards,
Jan

Quoted 13 lines

Other than that, this patch set looks good to me.
Best Regards,
Xiao Yang
Quoted 3 lines
include/tst_test.h +0 -0
  /* flush stderr and stdout */
  void tst_flush(void);

Petr Vorel

2018-12-04 17:35 UTC #4 lore raw

PATCH 2/2 lib: build check parameters for tst_brk()

Hi Jan,

Reviewed-by Petr Vorel <[email protected]>

Quoted 6 lines

Quoted 2 lines
Perhaps, we could add some hints about the invalid ttype. e.g. "tst_brk(): invalid type, please use TBROK/TCONF/TFAIL"
Do you have suggestion how to achieve that?
There's an __attribute__(error), but it's supported only from gcc 4.3 as I recall.

Do you mean __attribute__ format?

Quoted 1 line

Alternative would be link time failure, with a symbol name suggesting what went wrong.

Not sure, how exactly you want to do it, but seems to be more portable than requiring specific gcc version (although 4.3 is very old and __attribute__ format is supported by clang as well).

Kind regards,
Petr

Li Wang

2018-12-05 06:34 UTC #5 lore raw

PATCH 2/2 lib: build check parameters for tst_brk()

Quoted 10 lines · Petr Vorel

Hi Jan,
Reviewed-by Petr Vorel <[email protected]>
Quoted 4 lines
Quoted 2 lines
Perhaps, we could add some hints about the invalid ttype. e.g. "tst_brk(): invalid type, please use TBROK/TCONF/TFAIL"
Do you have suggestion how to achieve that?

Quoted 7 lines

Quoted 1 line
There's an __attribute__(error), but it's supported only from gcc 4.3 as I recall.
Do you mean __attribute__ format?
Quoted 1 line
Alternative would be link time failure, with a symbol name suggesting what went wrong.
Not sure, how exactly you want to do it, but seems to be more portable than requiring specific gcc version (although 4.3 is very old and __attribute__ format is supported by clang as well).

I took an rough look at the kernel method, maybe we can achieve that conditionally?

include/tst_common.h +19 -0

@@ -65,4 +65,23 @@
        ERET;                                                           \
 })
 
+#define GCC_VERSION (__GNUC__ * 10000          \
+                    + __GNUC_MINOR__ * 100     \
+                    + __GNUC_PATCHLEVEL__)
+
+#if GCC_VERSION >= 40300
+# define BUILD_BUG_ON_MSG(cond, msg) \
+       compiletime_assert(!(cond), msg, tst_brk_detect_)
+# define compiletime_assert(condition, msg, funcname_)         \
+       do {                                                    \
+               void funcname_(void) __attribute__((error(msg)));  \
+               if (!(condition))                               \
+                       funcname_();                            \
+       } while (0)
+#else
+# define BUILD_BUG_ON_MSG(cond, msg) BUILD_BUG_ON(cond)
+# define BUILD_BUG_ON(cond) \
+       do { ((void)sizeof(char[1 - 2 * !!(cond)])); } while (0)
+#endif
+
 #endif /* TST_COMMON_H__ */

include/tst_test.h +4 -2

index 2ebf746..03f3789 100644
@@ -69,8 +69,12 @@ void tst_brk_(const char *file, const int lineno, int ttype,
               const char *fmt, ...)
               __attribute__ ((format (printf, 4, 5)));
 
-#define tst_brk(ttype, arg_fmt, ...) \
-       tst_brk_(__FILE__, __LINE__, (ttype), (arg_fmt), ##__VA_ARGS__)
+#define tst_brk(ttype, arg_fmt, ...)
         \
+       ({
         \
+               BUILD_BUG_ON_MSG(!((ttype) & (TBROK | TCONF | TFAIL)), \
+                               "tst_brk(): invalid type, please use

TBROK/TCONF/TFAIL"); \
+ tst_brk_(__FILE__, __LINE__, (ttype), (arg_fmt),
##__VA_ARGS__);\
+ })

/* flush stderr and stdout */
void tst_flush(void);

Signature · 2 lines

Regards,
Li Wang

Petr Vorel

2018-12-05 09:25 UTC #6 lore raw

PATCH 2/2 lib: build check parameters for tst_brk()

Hi Li, Jan,

Quoted 11 lines

Quoted 4 lines
Quoted 1 line
Alternative would be link time failure, with a symbol name suggesting what went wrong.
Not sure, how exactly you want to do it, but seems to be more portable than requiring specific gcc version (although 4.3 is very old and __attribute__ format is supported by clang as well).
I took an rough look at the kernel method, maybe we can achieve that conditionally?

include/tst_common.h line 72

Quoted 8 lines

include/tst_common.h +5 -0

@@ -65,4 +65,23 @@
        ERET;                                                           \
 })
 
+#define GCC_VERSION (__GNUC__ * 10000          \
+                    + __GNUC_MINOR__ * 100     \
+                    + __GNUC_PATCHLEVEL__)
+
+#if GCC_VERSION >= 40300

Didn't you mean reverse?
#if GCC_VERSION < 40300

But this idea does not work for clang, which always use same version:
__GNUC__: 4
__GNUC_MINOR__: 2
__GNUC_PATCHLEVEL__: 1
(tested on clang 3.9, 5.0, 6.0)

There should be also CLANG_VERSION
__clang_major__
__clang_minor__
__clang_patchlevel__

Not sure, which clang version is compiled, even clang 4.0 works well:
https://travis-ci.org/pevik/ltp/builds/463734307
Maybe we could afford to skip check (searching for __clang__).

include/tst_common.h line 74

Quoted 2 lines

include/tst_common.h +2 -0

+# define BUILD_BUG_ON_MSG(cond, msg) \
+       compiletime_assert(!(cond), msg, tst_brk_detect_)

...

Kind regards,
Petr

Li Wang

2018-12-06 08:49 UTC #7 lore raw

PATCH 2/2 lib: build check parameters for tst_brk()

Quoted 17 lines · Petr Vorel

Hi Li, Jan,
Quoted 13 lines
Quoted 4 lines
Quoted 1 line
Alternative would be link time failure, with a symbol name suggesting what went wrong.
Not sure, how exactly you want to do it, but seems to be more portable than requiring specific gcc version (although 4.3 is very old and __attribute__ format is supported by clang as well).
I took an rough look at the kernel method, maybe we can achieve that conditionally?
include/tst_common.h +0 -0
@@ -65,4 +65,23 @@
        ERET;                                                           \
 })

Quoted 7 lines

include/tst_common.h line 72
Quoted 5 lines
include/tst_common.h +5 -0
+#define GCC_VERSION (__GNUC__ * 10000          \
+                    + __GNUC_MINOR__ * 100     \
+                    + __GNUC_PATCHLEVEL__)
+
+#if GCC_VERSION >= 40300
Didn't you mean reverse?
#if GCC_VERSION < 40300

No, I saw kernel gives this limitation so just use it as that.
See: https://github.com/torvalds/linux/blob/master/include/linux/build_bug.h#L55

Quoted 6 lines

But this idea does not work for clang, which always use same version:
__GNUC__: 4
__GNUC_MINOR__: 2
__GNUC_PATCHLEVEL__: 1
(tested on clang 3.9, 5.0, 6.0)

Noticed the Travis CI Build #860 works fine with clang, did you config anything special for that? BTW, the remain error part of GCC is not the same issue here, see below comments.

Build #860:
https://travis-ci.org/pevik/ltp/builds/463720369
https://github.com/pevik/ltp/commit/643bceea36c3447add142fcb5e7a3f79e9ac65a2

Quoted 9 lines

There should be also CLANG_VERSION
__clang_major__
__clang_minor__
__clang_patchlevel__
Not sure, which clang version is compiled, even clang 4.0 works well:
https://travis-ci.org/pevik/ltp/builds/463734307
Maybe we could afford to skip check (searching for __clang__).

Per GCC compiling error, seems the root cause is tst_brk() parameter(tst_test.c: line#355) not using in demanded, isn't that what we expected?

In function ‘check_child_status’,
inlined from ‘tst_reap_children’ at tst_test.c:371:4:
../include/tst_common.h:74:41: error: call to ‘tst_brk_detect_’ declared with attribute error: tst_brk(): invalid type, please use TBROK/TCONF/TFAIL
compiletime_assert(!(cond), msg, tst_brk_detect_)
^
../include/tst_common.h:79:24: note: in definition of macro ‘compiletime_assert’

                        funcname_();                            \
                        ^
../include/tst_test.h:74:16: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
                BUILD_BUG_ON_MSG(!((ttype) & (TBROK | TCONF | TFAIL)), \
                ^

tst_test.c:355:3: note: in expansion of macro ‘tst_brk’
tst_brk(ret, "Reported by child (%i)", pid);
^
make[1]: *** [tst_test.o] Error 1
make[1]: *** Waiting for unfinished jobs.... make[1]: Leaving directory `/home/travis/build/pevik/ltp/lib'
make: *** [lib-all] Error 2

--
Regards,
Li Wang

Xiao Yang

2018-12-06 09:19 UTC #8 lore raw

PATCH 2/2 lib: build check parameters for tst_brk()

Hi all,

Sorry, i forgot to reply this mail. :-(

Quoted 42 lines · Li Wang

Quoted 6 lines · Petr Vorel
Hi Li, Jan,
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
Alternative would be link time failure, with a symbol name suggesting what went wrong.
Not sure, how exactly you want to do it, but seems to be more portable than requiring specific gcc version (although 4.3 is very old and __attribute__ format is supported by clang as well).
Quoted 14 lines
include/tst_common.h line 71
Quoted 12 lines
I took an rough look at the kernel method, maybe we can achieve that conditionally?
include/tst_common.h +5 -0
@@ -65,4 +65,23 @@
         ERET;                                                           \
  })
+#define GCC_VERSION (__GNUC__ * 10000          \
+                    + __GNUC_MINOR__ * 100     \
+                    + __GNUC_PATCHLEVEL__)
+
+#if GCC_VERSION>= 40300
Didn't you mean reverse?
#if GCC_VERSION< 40300
No, I saw kernel gives this limitation so just use it as that.
See: https://github.com/torvalds/linux/blob/master/include/linux/build_bug.h#L55
Quoted 5 lines
But this idea does not work for clang, which always use same version:
__GNUC__: 4
__GNUC_MINOR__: 2
__GNUC_PATCHLEVEL__: 1
(tested on clang 3.9, 5.0, 6.0)
Noticed the Travis CI Build #860 works fine with clang, did you config anything special for that? BTW, the remain error part of GCC is not the same issue here, see below comments.
Build #860:
https://travis-ci.org/pevik/ltp/builds/463720369
https://github.com/pevik/ltp/commit/643bceea36c3447add142fcb5e7a3f79e9ac65a2
Quoted 5 lines
There should be also CLANG_VERSION
__clang_major__
__clang_minor__
__clang_patchlevel__

Quoted 6 lines

Quoted 3 lines
Not sure, which clang version is compiled, even clang 4.0 works well:
https://travis-ci.org/pevik/ltp/builds/463734307
Maybe we could afford to skip check (searching for __clang__).
Per GCC compiling error, seems the root cause is tst_brk() parameter(tst_test.c: line#355) not using in demanded, isn't that what we expected?

I think we should just check invalid type for constants.

Quoted 16 lines

In function ‘check_child_status’,
inlined from ‘tst_reap_children’ at tst_test.c:371:4:
../include/tst_common.h:74:41: error: call to ‘tst_brk_detect_’ declared with attribute error: tst_brk(): invalid type, please use TBROK/TCONF/TFAIL
compiletime_assert(!(cond), msg, tst_brk_detect_)
^
../include/tst_common.h:79:24: note: in definition of macro ‘compiletime_assert’
                         funcname_();                            \
                         ^
../include/tst_test.h:74:16: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
                 BUILD_BUG_ON_MSG(!((ttype)&  (TBROK | TCONF | TFAIL)), \
                 ^
tst_test.c:355:3: note: in expansion of macro ‘tst_brk’
    tst_brk(ret, "Reported by child (%i)", pid);
    ^

Quoted 5 lines

make[1]: *** [tst_test.o] Error 1
make[1]: *** Waiting for unfinished jobs.... make[1]: Leaving directory `/home/travis/build/pevik/ltp/lib'
make: *** [lib-all] Error 2

Perhaps, we need to distinguish between constants and variables by __builtin_constant_p().

Best Regards,
Xiao Yang

Quoted 4 lines

--
Regards,
Li Wang

Li Wang

2018-12-06 10:15 UTC #9 lore raw

PATCH 2/2 lib: build check parameters for tst_brk()

Quoted 1 line · Xiao Yang

Perhaps, we need to distinguish between constants and variables by __builtin_constant_p().

Hmm, I don't think so. If someone pass 'TINFO/TPASS/TTYPO' via variable, the tst_brk() parameter check will be missed there.

Signature · 2 lines

Regards,
Li Wang

Li Wang

2018-12-06 10:33 UTC #10 lore raw

PATCH 2/2 lib: build check parameters for tst_brk()

In reply to Li Wang

Hi Petr,

Quoted 6 lines · Li Wang

Quoted 5 lines
But this idea does not work for clang, which always use same version:
__GNUC__: 4
__GNUC_MINOR__: 2
__GNUC_PATCHLEVEL__: 1
(tested on clang 3.9, 5.0, 6.0)

Quoted 7 lines

Noticed the Travis CI Build #860 works fine with clang, did you config anything special for that? BTW, the remain error part of GCC is not the same issue here, see below comments.
Build #860:
https://travis-ci.org/pevik/ltp/builds/463720369
https://github.com/pevik/ltp/commit/643bceea36c3447add142fcb5e7a3f79e9ac65a2

Can you help to test this commit with same configuration in build#860? (I haven't mastered how to use Travis CI yet)
https://github.com/wangli5665/ltp/commit/bbf236875c11861e40845edd875e517dd475bfac

I used gcc version 4.1.2 and didn't hit the un-supportted issue locally, I'm inclined to suspect there's no demands on gcc compiler.

Signature · 2 lines

Regards,
Li Wang

Petr Vorel

2018-12-06 12:59 UTC #11 lore raw

PATCH 2/2 lib: build check parameters for tst_brk()

Hi Li,

Quoted 3 lines

Can you help to test this commit with same configuration in build#860? (I haven't mastered how to use Travis CI yet)
https://github.com/wangli5665/ltp/commit/bbf236875c11861e40845edd875e517dd475bfac

Sure. This commit is on
https://travis-ci.org/pevik/ltp/builds/464316410
=> failing on
../../../../include/tst_common.h:72:35: error: call to ‘check_tst_brk_parameter_’ declared with attribute error: tst_brk(): invalid type, please use TBROK/TCONF/TFAIL

+ your branch check-tst_brk-parameter (with code cleanup commit)
https://github.com/wangli5665/ltp/commits/check-tst_brk-parameter
is on
https://travis-ci.org/pevik/ltp/builds/464316329
=> the same failure:
../../../../include/tst_common.h:72:35: warning: implicit declaration of function ‘check_tst_brk_parameter_’ [-Wimplicit-function-declaration]

Travis is not complicated: 1) register 2) give it rights to your account (I don't like this) 3) enable ltp repository.
Every push since it it's triggered.

Quoted 2 lines

I used gcc version 4.1.2 and didn't hit the un-supportted issue locally, I'm inclined to suspect there's no demands on gcc compiler.

The oldest gcc I have available is 4.3.

Kind regards,
Petr

Petr Vorel

2018-12-06 13:07 UTC #12 lore raw

PATCH 2/2 lib: build check parameters for tst_brk()

In reply to Li Wang

Hi Li,

Quoted 6 lines

Quoted 3 lines
Quoted 1 line
+#if GCC_VERSION >= 40300
Didn't you mean reverse?
#if GCC_VERSION < 40300
No, I saw kernel gives this limitation so just use it as that.
See: https://github.com/torvalds/linux/blob/master/include/linux/build_bug.h#L55

OK, I didn't get from Jan's "There's an __attribute__(error), but it's supported only from gcc 4.3 as I recall.", that it is in 4.3 and older. First I thought it's a new feature, but didn't find any reference.

Quoted 9 lines

Quoted 5 lines
But this idea does not work for clang, which always use same version:
__GNUC__: 4
__GNUC_MINOR__: 2
__GNUC_PATCHLEVEL__: 1
(tested on clang 3.9, 5.0, 6.0)
Noticed the Travis CI Build #860 works fine with clang, did you config anything special for that? BTW, the remain error part of GCC is not the same issue here, see below comments.

No.

Quoted 3 lines

Build #860:
https://travis-ci.org/pevik/ltp/builds/463720369
https://github.com/pevik/ltp/commit/643bceea36c3447add142fcb5e7a3f79e9ac65a2

It's your commit from https://patchwork.ozlabs.org/patch/995203/#2045049

+ syscalls/mq: don't use TWARN with tst_brk()
https://patchwork.ozlabs.org/patch/995202/

Kind regards,
Petr

Li Wang

2018-12-07 06:28 UTC #13 lore raw

PATCH 2/2 lib: build check parameters for tst_brk()

Hi Petr,

Quoted 5 lines · Petr Vorel

Quoted 2 lines
No, I saw kernel gives this limitation so just use it as that.
See: https://github.com/torvalds/linux/blob/master/include/linux/build_bug.h#L55
OK, I didn't get from Jan's "There's an __attribute__(error), but it's supported only from gcc 4.3 as I recall.", that it is in 4.3 and older. First I thought it's a new feature, but didn't find any reference.

I have tried on Travis CI for many new commits build, I didn't found GCC(even gcc-4.1.2) outthrow warning in the compiling phase. But seems only clang(3.9, 4.0, 5.0) blames '__error__' is unknown attribute. Maybe we should consider to check clang version, but currently I have no idea which version should be set as baseline.

https://travis-ci.org/wangli5665/ltp/builds
https://github.com/wangli5665/ltp/blob/check-tst_brk-parameter/include/tst_common.h#L68

Signature · 2 lines

Regards,
Li Wang

Cyril Hrubis

2018-12-11 14:58 UTC #14 lore raw

PATCH 2/2 lib: build check parameters for tst_brk()

In reply to Jan Stancek

Hi!

Quoted 1 line

Do you have suggestion how to achieve that?

We can always name the BUILD_BUG_ON() macro to be TST_BRK_ONLY_SUPPORTS_TCONF_TBROK_TFAIL() and be done with it.

I would rather avoided playing with specific compiler features etc. So this version looks good to me.

Signature · 2 lines

Cyril Hrubis
[email protected]

Jan Stancek

author

2019-01-03 11:52 UTC #15 lore raw

PATCH 2/2 lib: build check parameters for tst_brk()

----- Original Message -----

Quoted 8 lines

Hi!
Quoted 1 line
Do you have suggestion how to achieve that?
We can always name the BUILD_BUG_ON() macro to be TST_BRK_ONLY_SUPPORTS_TCONF_TBROK_TFAIL() and be done with it.
I would rather avoided playing with specific compiler features etc. So this version looks good to me.

Pushed the original version for now.

Regards,
Jan

Cyril Hrubis

2019-01-07 18:25 UTC #16 lore raw

PATCH 2/2 lib: build check parameters for tst_brk()

Hi!

Quoted 7 lines

Quoted 5 lines
We can always name the BUILD_BUG_ON() macro to be TST_BRK_ONLY_SUPPORTS_TCONF_TBROK_TFAIL() and be done with it.
I would rather avoided playing with specific compiler features etc. So this version looks good to me.
Pushed the original version for now.

FYI looks like we need to rename the BUILD_BUG_ON() after all:

gcc -g -O2 -g -O2 -fno-strict-aliasing -pipe -Wall -W -Wold-style-definition -Illtp -o quotactl03 In file included from ../../../../include/tst_test.h:29:0,

                 from quotactl03.c:53:

../../../../include/tst_common.h:68:0: warning: "BUILD_BUG_ON" redefined
#define BUILD_BUG_ON(condition) \

In file included from /usr/include/xfs/xqm.h:21:0,

                 from quotactl03.c:50:

/usr/include/xfs/xfs.h:68:0: note: this is the location of the previous definiti
#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))

gcc -g -O2 -g -O2 -fno-strict-aliasing -pipe -Wall -W -Wold-style-definition -Illtp -o quotactl01 In file included from quotactl01.c:56:0:
quotactl01.c: In function 'setup':
../../../../include/tst_test.h:85:3: warning: this statement may fall through [-
tst_brk_(__FILE__, __LINE__, (ttype), (arg_fmt), ##__VA_ARGS__);\
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
quotactl01.c:169:3: note: in expansion of macro 'tst_brk'
tst_brk(TCONF, "quotacheck binary not installed");
^~~~~~~
quotactl01.c:170:2: note: here
default:
^~~~~~~

Signature · 2 lines

Cyril Hrubis
[email protected]

Jan Stancek

author

2019-01-07 19:06 UTC #17 lore raw

PATCH 2/2 lib: build check parameters for tst_brk()

----- Original Message -----

Quoted 10 lines

Hi!
Quoted 7 lines
Quoted 5 lines
We can always name the BUILD_BUG_ON() macro to be TST_BRK_ONLY_SUPPORTS_TCONF_TBROK_TFAIL() and be done with it.
I would rather avoided playing with specific compiler features etc. So this version looks good to me.
Pushed the original version for now.
FYI looks like we need to rename the BUILD_BUG_ON() after all:

LTP_BUILD_BUG_ON() or do you have something other in mind?

Quoted 34 lines

gcc -g -O2 -g -O2 -fno-strict-aliasing -pipe -Wall -W -Wold-style-definition -Illtp -o quotactl03
In file included from ../../../../include/tst_test.h:29:0,
                 from quotactl03.c:53:
../../../../include/tst_common.h:68:0: warning: "BUILD_BUG_ON" redefined
#define BUILD_BUG_ON(condition) \
In file included from /usr/include/xfs/xqm.h:21:0,
                 from quotactl03.c:50:
/usr/include/xfs/xfs.h:68:0: note: this is the location of the previous definiti
#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
gcc -g -O2 -g -O2 -fno-strict-aliasing -pipe -Wall -W -Wold-style-definition -Illtp -o quotactl01
In file included from quotactl01.c:56:0:
quotactl01.c: In function 'setup':
../../../../include/tst_test.h:85:3: warning: this statement may fall through [-
   tst_brk_(__FILE__, __LINE__, (ttype), (arg_fmt), ##__VA_ARGS__);\
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
quotactl01.c:169:3: note: in expansion of macro 'tst_brk'
   tst_brk(TCONF, "quotacheck binary not installed");
   ^~~~~~~
quotactl01.c:170:2: note: here
default:
^~~~~~~
--
Cyril Hrubis
[email protected]

Cyril Hrubis

2019-01-07 19:22 UTC #18 lore raw

PATCH 2/2 lib: build check parameters for tst_brk()

Hi!

Quoted 11 lines

Quoted 9 lines
Quoted 7 lines
Quoted 5 lines
We can always name the BUILD_BUG_ON() macro to be TST_BRK_ONLY_SUPPORTS_TCONF_TBROK_TFAIL() and be done with it.
I would rather avoided playing with specific compiler features etc. So this version looks good to me.
Pushed the original version for now.
FYI looks like we need to rename the BUILD_BUG_ON() after all:
LTP_BUILD_BUG_ON() or do you have something other in mind?

Most of the new library has TST_ prefix, but we can still rename it to TST_BRK_SUPPORTS_ONLY_TCONF_TBROK()...

Signature · 2 lines

Cyril Hrubis
[email protected]

Jan Stancek

author

2019-01-08 11:14 UTC #19 lore raw

PATCH 2/2 lib: build check parameters for tst_brk()

----- Original Message -----

Quoted 8 lines

Hi!
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
We can always name the BUILD_BUG_ON() macro to be TST_BRK_ONLY_SUPPORTS_TCONF_TBROK_TFAIL() and be done with it.
I would rather avoided playing with specific compiler features etc. So
this version looks good to me.

Quoted 8 lines

Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Pushed the original version for now.
FYI looks like we need to rename the BUILD_BUG_ON() after all:
LTP_BUILD_BUG_ON() or do you have something other in mind?
Most of the new library has TST_ prefix, but we can still rename it to TST_BRK_SUPPORTS_ONLY_TCONF_TBROK()...

OK, I pushed patch that renames it to above.

Quoted 5 lines