| Message ID | d3ce5d3247f7f09cfabd288b40893bb631fb190f.1541710635.git.jstancek@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/2] syscalls/mq: don't use TWARN with tst_brk() | expand |
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);
----- 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); > > > >
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
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);
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
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
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 >
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.
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.
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
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
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
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.
----- 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
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: ^~~~~~~
----- 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 >
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()...
----- 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 >
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);
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(-)