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

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

[Jan Stancek]

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 testcases/kernel/syscalls/utils/mq.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/utils/mq.h b/testcases/kernel/syscalls/utils/mq.h
index 0bdac97229cd..b55228c885dc 100644
--- a/testcases/kernel/syscalls/utils/mq.h
+++ b/testcases/kernel/syscalls/utils/mq.h
@@ -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;
 	}
 
-- 
1.8.3.1

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

[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 <jstancek@redhat.com>
---
 include/tst_common.h | 3 +++
 include/tst_test.h   | 7 +++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/tst_common.h b/include/tst_common.h
index 27924766ef6e..358f2a76ecda 100644
--- a/include/tst_common.h
+++ b/include/tst_common.h
@@ -65,4 +65,7 @@
 	ERET;								\
 })
 
+#define BUILD_BUG_ON(condition) \
+	do { ((void)sizeof(char[1 - 2 * !!(condition)])); } while (0)
+
 #endif /* TST_COMMON_H__ */
diff --git a/include/tst_test.h b/include/tst_test.h
index 2ebf746eb720..cd936eb792bd 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -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);
-- 
1.8.3.1

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

[Xiao Yang]

On 2018/11/09 4:59, Jan Stancek wrote:
> 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<jstancek@redhat.com>
> ---
>   include/tst_common.h | 3 +++
>   include/tst_test.h   | 7 +++++--
>   2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/include/tst_common.h b/include/tst_common.h
> index 27924766ef6e..358f2a76ecda 100644
> --- a/include/tst_common.h
> +++ b/include/tst_common.h
> @@ -65,4 +65,7 @@
>   	ERET;								\
>   })
>
> +#define BUILD_BUG_ON(condition) \
> +	do { ((void)sizeof(char[1 - 2 * !!(condition)])); } while (0)
> +
>   #endif /* TST_COMMON_H__ */
> diff --git a/include/tst_test.h b/include/tst_test.h
> index 2ebf746eb720..cd936eb792bd 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -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

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

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

[Jan Stancek]

----- Original Message -----
> On 2018/11/09 4:59, Jan Stancek wrote:
> > 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<jstancek@redhat.com>
> > ---
> >   include/tst_common.h | 3 +++
> >   include/tst_test.h   | 7 +++++--
> >   2 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/tst_common.h b/include/tst_common.h
> > index 27924766ef6e..358f2a76ecda 100644
> > --- a/include/tst_common.h
> > +++ b/include/tst_common.h
> > @@ -65,4 +65,7 @@
> >   	ERET;								\
> >   })
> >
> > +#define BUILD_BUG_ON(condition) \
> > +	do { ((void)sizeof(char[1 - 2 * !!(condition)])); } while (0)
> > +
> >   #endif /* TST_COMMON_H__ */
> > diff --git a/include/tst_test.h b/include/tst_test.h
> > index 2ebf746eb720..cd936eb792bd 100644
> > --- a/include/tst_test.h
> > +++ b/include/tst_test.h
> > @@ -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"

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

> 
> Other than that, this patch set looks good to me.
> 
> Best Regards,
> Xiao Yang
> 
> >
> >   /* flush stderr and stdout */
> >   void tst_flush(void);
> 
> 
> 
> 

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

[Petr Vorel]

Hi Jan,

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

> > 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?

> 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

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

[Li Wang]

On Wed, Dec 5, 2018 at 1:35 AM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Jan,
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> > > 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?
>
> > 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?

--- a/include/tst_common.h
+++ b/include/tst_common.h
@@ -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__ */
diff --git a/include/tst_test.h b/include/tst_test.h
index 2ebf746..03f3789 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -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);

-- 
Regards,
Li Wang

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

[Petr Vorel]

Hi Li, Jan,

> > > 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?

> --- a/include/tst_common.h
> +++ b/include/tst_common.h
> @@ -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__).

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


Kind regards,
Petr

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

[Li Wang]

On Wed, Dec 5, 2018 at 5:25 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Li, Jan,
>
> > > > 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?
>
> > --- a/include/tst_common.h
> > +++ b/include/tst_common.h
> > @@ -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

>
> 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

>
> 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

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

[Xiao Yang]

Hi all,

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

On 2018/12/06 16:49, Li Wang wrote:
> On Wed, Dec 5, 2018 at 5:25 PM Petr Vorel<pvorel@suse.cz>  wrote:
>> Hi Li, Jan,
>>
>>>>> 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?
>>> --- a/include/tst_common.h
>>> +++ b/include/tst_common.h
>>> @@ -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
>
>> 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
>
>> 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?
I think we should just check invalid type for constants.

> 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
>
Perhaps, we need to distinguish between constants and variables by __builtin_constant_p().

Best Regards,
Xiao Yang

> --
> Regards,
> Li Wang
>

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

[Li Wang]

Xiao Yang <yangx.jy@cn.fujitsu.com> wrote:

> 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.

-- 
Regards,
Li Wang

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

[Li Wang]

Hi Petr,

On Thu, Dec 6, 2018 at 4:49 PM Li Wang <liwang@redhat.com> wrote:

> > 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

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.

-- 
Regards,
Li Wang

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

[Petr Vorel]

Hi Li,

> 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.

> 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

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

[Petr Vorel]

Hi Li,

> > > +#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.


> > 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.

> 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

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

[Li Wang]

Hi Petr,

On Thu, Dec 6, 2018 at 9:07 PM Petr Vorel <pvorel@suse.cz> wrote:

> > 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

-- 
Regards,
Li Wang

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

[Cyril Hrubis]

Hi!
> 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.

-- 
Cyril Hrubis
chrubis@suse.cz

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

[Jan Stancek]

----- Original Message -----
> Hi!
> > 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

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

[Cyril Hrubis]

Hi!
> > 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:
  ^~~~~~~


-- 
Cyril Hrubis
chrubis@suse.cz

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

[Jan Stancek]

----- Original Message -----
> Hi!
> > > 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?

> 
> 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
> chrubis@suse.cz
> 

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

[Cyril Hrubis]

Hi!
> > > > 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()...

-- 
Cyril Hrubis
chrubis@suse.cz

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

[Jan Stancek]

----- Original Message -----
> Hi!
> > > > > 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()...

OK, I pushed patch that renames it to above.

> 
> --
> Cyril Hrubis
> chrubis@suse.cz
> 

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

[Petr Vorel]

Hi Jan,

Acked-by: Petr Vorel <pvorel@suse.cz>


Kind regards,
Petr

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

[Cyril Hrubis]

Hi!
IMHO this is obvious enough to go in without review...

You can add my ack here nevertheless.

-- 
Cyril Hrubis
chrubis@suse.cz