| Message ID | 20241203151530.16882-5-chrubis@suse.cz |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | First new shell library converted test | expand |
> We use the tst_res_ helper for tst_brk_ as well so we need to be able to > handle TBROK type as well. How can we call tst_brk_() via tst_res_() ? tst_res_(argv[1], atoi(argv[2]), type, "%s", msg); Also we have TST_RES_SUPPORTS_TCONF_TDEBUG_TFAIL_TINFO_TPASS_TWARN check to not add TBROK to tst_res_(). ... > - else if (!strcmp(argv[3], "TINFO")) > + } else if (!strcmp(argv[3], "TINFO")) { > type = TINFO; > - else if (!strcmp(argv[3], "TDEBUG")) > + } else if (!strcmp(argv[3], "TDEBUG")) { > type = TDEBUG; > - else > + } else if (!strcmp(argv[3], "TBROK")) { > + type = TBROK; > + } else { > + printf("Wrong type '%s'\n", argv[3]); > goto help; Also tst_brk allows only TBROK and TCONF in C: #define tst_brk(ttype, arg_fmt, ...) \ ({ \ TST_BRK_SUPPORTS_ONLY_TCONF_TBROK(!((ttype) & \ (TBROK | TCONF | TFAIL))); \ tst_brk_(__FILE__, __LINE__, (ttype), (arg_fmt), ##__VA_ARGS__);\ }) ... and shell: if [ "$res" != TBROK -a "$res" != TCONF ]; then tst_res TBROK "tst_brk can be called only with TBROK or TCONF ($res)" else tst_res "$res" "$@" fi Kind regards, Petr
Hi! > > We use the tst_res_ helper for tst_brk_ as well so we need to be able to > > handle TBROK type as well. > > How can we call tst_brk_() via tst_res_() ? > tst_res_(argv[1], atoi(argv[2]), type, "%s", msg); In the end both of these functions increment counters, but in this case we need to return to the shell so we cannot call tst_brk() in the helper. It's a very special situation here. > Also we have TST_RES_SUPPORTS_TCONF_TDEBUG_TFAIL_TINFO_TPASS_TWARN check to not > add TBROK to tst_res_(). That only works when the value is constant, if you pass via variable that is not constant at build time you can pass whatever you want. Which is another reason why it makes sense to relax the constraints. > ... > > - else if (!strcmp(argv[3], "TINFO")) > > + } else if (!strcmp(argv[3], "TINFO")) { > > type = TINFO; > > - else if (!strcmp(argv[3], "TDEBUG")) > > + } else if (!strcmp(argv[3], "TDEBUG")) { > > type = TDEBUG; > > - else > > + } else if (!strcmp(argv[3], "TBROK")) { > > + type = TBROK; > > + } else { > > + printf("Wrong type '%s'\n", argv[3]); > > goto help; > > Also tst_brk allows only TBROK and TCONF in C: > > #define tst_brk(ttype, arg_fmt, ...) \ > ({ \ > TST_BRK_SUPPORTS_ONLY_TCONF_TBROK(!((ttype) & \ > (TBROK | TCONF | TFAIL))); \ > tst_brk_(__FILE__, __LINE__, (ttype), (arg_fmt), ##__VA_ARGS__);\ > }) See above.
Hi Cyril, > Hi! > > > We use the tst_res_ helper for tst_brk_ as well so we need to be able to > > > handle TBROK type as well. > > How can we call tst_brk_() via tst_res_() ? > > tst_res_(argv[1], atoi(argv[2]), type, "%s", msg); > In the end both of these functions increment counters, but in this case > we need to return to the shell so we cannot call tst_brk() in the > helper. It's a very special situation here. Thanks for info. Maybe a little note anywhere (e.g. commit message) would help. > > Also we have TST_RES_SUPPORTS_TCONF_TDEBUG_TFAIL_TINFO_TPASS_TWARN check to not > > add TBROK to tst_res_(). > That only works when the value is constant, if you pass via variable > that is not constant at build time you can pass whatever you want. Which > is another reason why it makes sense to relax the constraints. Good point. So you plan to remove these build time checks? Also we have nice docs from you include/tst_res_flags.h. ATM include/tst_test.h and doc/old/C-Test-API.asciidoc are outdated, but if you relax allowed ttype, than it would not need to be updated. Also, any reason to not support TWARN? I suppose we don't need TERRNO, TTERRNO, TRERRNO (not supported by tst_test.sh). Anyway, LGTM. Reviewed-by: Petr Vorel <pvorel@suse.cz> Kind regards, Petr
Hi! > > In the end both of these functions increment counters, but in this case > > we need to return to the shell so we cannot call tst_brk() in the > > helper. It's a very special situation here. > > Thanks for info. Maybe a little note anywhere (e.g. commit message) would help. Will do. > > > Also we have TST_RES_SUPPORTS_TCONF_TDEBUG_TFAIL_TINFO_TPASS_TWARN check to not > > > add TBROK to tst_res_(). > > > That only works when the value is constant, if you pass via variable > > that is not constant at build time you can pass whatever you want. Which > > is another reason why it makes sense to relax the constraints. > > Good point. So you plan to remove these build time checks? For tst_brk() these checks are removed in my patchset that changes how TBROK is propagated, so that tst_brk() works with TFAIL and TPASS as well. We also had a discussion about removing TWARN since there is not a big difference between TBROK and TWARN and replacing TWARN with tst_res(TBROK, ...) as that would make things simpler. Currently there is no good rule where to use TBROK and where TWARN so removing TWARN sounds like a good option. > Also we have nice docs from you include/tst_res_flags.h. > ATM include/tst_test.h and doc/old/C-Test-API.asciidoc are outdated, > but if you relax allowed ttype, than it would not need to be updated. > > Also, any reason to not support TWARN? See above. > I suppose we don't need TERRNO, TTERRNO, TRERRNO (not supported by tst_test.sh). Yes, errno is not defined in shell.
diff --git a/testcases/lib/tst_res_.c b/testcases/lib/tst_res_.c index a43920f36..fd9b8e841 100644 --- a/testcases/lib/tst_res_.c +++ b/testcases/lib/tst_res_.c @@ -8,28 +8,34 @@ static void print_help(void) { - printf("Usage: tst_res_ filename lineno [TPASS|TFAIL|TCONF|TINFO|TDEBUG] 'A short description'\n"); + printf("Usage: tst_{res,brk} filename lineno [TPASS|TBROK|TFAIL|TCONF|TINFO|TDEBUG] 'A short description'\n"); } int main(int argc, char *argv[]) { int type, i; - if (argc < 5) + if (argc < 5) { + printf("argc = %i expected 5\n", argc); goto help; + } - if (!strcmp(argv[3], "TPASS")) + if (!strcmp(argv[3], "TPASS")) { type = TPASS; - else if (!strcmp(argv[3], "TFAIL")) + } else if (!strcmp(argv[3], "TFAIL")) { type = TFAIL; - else if (!strcmp(argv[3], "TCONF")) + } else if (!strcmp(argv[3], "TCONF")) { type = TCONF; - else if (!strcmp(argv[3], "TINFO")) + } else if (!strcmp(argv[3], "TINFO")) { type = TINFO; - else if (!strcmp(argv[3], "TDEBUG")) + } else if (!strcmp(argv[3], "TDEBUG")) { type = TDEBUG; - else + } else if (!strcmp(argv[3], "TBROK")) { + type = TBROK; + } else { + printf("Wrong type '%s'\n", argv[3]); goto help; + } size_t len = 0;
We use the tst_res_ helper for tst_brk_ as well so we need to be able to handle TBROK type as well. Signed-off-by: Cyril Hrubis <chrubis@suse.cz> --- testcases/lib/tst_res_.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-)