| Message ID | 1525663451-32016-1-git-send-email-yangx.jy@cn.fujitsu.com |
|---|---|
| State | Rejected |
| Delegated to: | Petr Vorel |
| Headers | show |
| Series | [1/3] lib/tst_net.sh: Append 6 to the end of $TST_OPTS | expand |
Hi, > If the first character of optstring is set to a colon(tcp_fastopen_run.sh, > nfs_lib.sh), getopts should be in silent mode rather than process it as > an argument of 6. > Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com> > --- > testcases/lib/tst_net.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh > index 3a0fe01..32b4f09 100644 > --- a/testcases/lib/tst_net.sh > +++ b/testcases/lib/tst_net.sh > @@ -19,7 +19,7 @@ > # Author: Alexey Kodanev <alexey.kodanev@oracle.com> > -TST_OPTS="6$TST_OPTS" > +TST_OPTS="${TST_OPTS}6" > TST_PARSE_ARGS_CALLER="$TST_PARSE_ARGS" > TST_PARSE_ARGS="tst_net_parse_args" > TST_USAGE_CALLER="$TST_USAGE" Acked-by: Petr Vorel <pvorel@suse.cz> Good catch. Although I propose to get rid of ':' at the beginning for users of new shell API (as it's IMHO better to see errors). Kind regards, Petr
On 2018/05/07 14:43, Petr Vorel wrote: > Hi, > >> If the first character of optstring is set to a colon(tcp_fastopen_run.sh, >> nfs_lib.sh), getopts should be in silent mode rather than process it as >> an argument of 6. >> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com> >> --- >> testcases/lib/tst_net.sh | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh >> index 3a0fe01..32b4f09 100644 >> --- a/testcases/lib/tst_net.sh >> +++ b/testcases/lib/tst_net.sh >> @@ -19,7 +19,7 @@ >> # Author: Alexey Kodanev<alexey.kodanev@oracle.com> > >> -TST_OPTS="6$TST_OPTS" >> +TST_OPTS="${TST_OPTS}6" >> TST_PARSE_ARGS_CALLER="$TST_PARSE_ARGS" >> TST_PARSE_ARGS="tst_net_parse_args" >> TST_USAGE_CALLER="$TST_USAGE" > Acked-by: Petr Vorel<pvorel@suse.cz> > > Good catch. Although I propose to get rid of ':' at the beginning for users of new shell > API (as it's IMHO better to see errors). Hi Petr, If getopts is in silent mode, the invalid option is placed in OPTARG, so we can see errors by printing the value of OPTARG. But it is reasonable for me to get rid of ':' at the beginning of optstring. :-) Thanks, Xiao Yang > > Kind regards, > Petr > > > . >
Hi Xiao, > > Good catch. Although I propose to get rid of ':' at the beginning for users of new shell > > API (as it's IMHO better to see errors). > Hi Petr, > If getopts is in silent mode, the invalid option is placed in OPTARG, so > we can see errors by printing > the value of OPTARG. But it is reasonable for me to get rid of ':' at the > beginning of optstring. :-) I know, TBROK inform us, so it can stay how it is for legacy API. I meant it for the new API, where is going to be removed (the TBROK as well) as error is handled in tst_run in tst_test.sh. > Thanks, > Xiao Yang Kind regards, Petr Vorel
On 05/07/2018 11:09 AM, Petr Vorel wrote: > Hi Xiao, > >>> Good catch. Although I propose to get rid of ':' at the beginning for users of new shell >>> API (as it's IMHO better to see errors). >> Hi Petr, > >> If getopts is in silent mode, the invalid option is placed in OPTARG, so >> we can see errors by printing >> the value of OPTARG. But it is reasonable for me to get rid of ':' at the >> beginning of optstring. :-) > I know, TBROK inform us, so it can stay how it is for legacy API. > I meant it for the new API, where is going to be removed (the TBROK as well) as error is > handled in tst_run in tst_test.sh. > What about silence error in tst_test.sh? And remove preceding column in the tests (tcp_fastopen.sh, nfs_lib.sh). We handle the error already there, in tst_run (and in old API the scripts have TBROK), so there is no point printing one more redundant message that doesn't have LTP format. diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh index 8d49d34..edac619 100644 --- a/testcases/lib/tst_test.sh +++ b/testcases/lib/tst_test.sh @@ -265,11 +265,12 @@ tst_run() OPTIND=1 - while getopts "hi:$TST_OPTS" name $TST_ARGS; do + while getopts ":hi:$TST_OPTS" name $TST_ARGS; do case $name in 'h') tst_usage; exit 0;; 'i') TST_ITERATIONS=$OPTARG;; - '?') tst_usage; exit 2;; + '?') tst_usage + tst_brk TBROK "invalid option: '$OPTARG'";; *) $TST_PARSE_ARGS "$name" "$OPTARG";; esac
Hi, > On 05/07/2018 11:09 AM, Petr Vorel wrote: > > Hi Xiao, > >>> Good catch. Although I propose to get rid of ':' at the beginning for users of new shell > >>> API (as it's IMHO better to see errors). > >> Hi Petr, > >> If getopts is in silent mode, the invalid option is placed in OPTARG, so > >> we can see errors by printing > >> the value of OPTARG. But it is reasonable for me to get rid of ':' at the > >> beginning of optstring. :-) > > I know, TBROK inform us, so it can stay how it is for legacy API. > > I meant it for the new API, where is going to be removed (the TBROK as well) as error is > > handled in tst_run in tst_test.sh. > What about silence error in tst_test.sh? And remove preceding column in the > tests (tcp_fastopen.sh, nfs_lib.sh). We handle the error already there, in > tst_run (and in old API the scripts have TBROK), so there is no point > printing one more redundant message that doesn't have LTP format. > diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh > index 8d49d34..edac619 100644 > --- a/testcases/lib/tst_test.sh > +++ b/testcases/lib/tst_test.sh > @@ -265,11 +265,12 @@ tst_run() > OPTIND=1 > - while getopts "hi:$TST_OPTS" name $TST_ARGS; do > + while getopts ":hi:$TST_OPTS" name $TST_ARGS; do > case $name in > 'h') tst_usage; exit 0;; > 'i') TST_ITERATIONS=$OPTARG;; > - '?') tst_usage; exit 2;; > + '?') tst_usage > + tst_brk TBROK "invalid option: '$OPTARG'";; > *) $TST_PARSE_ARGS "$name" "$OPTARG";; > esac +1 as it makes sense to unify things. It's just a bit faster to have printed the name of the script (if we don't use ':'). BTW this change affect only tests using new API (i.e. not tcp_fastopen_run.sh, nor nfs_lib.sh which still use legacy API). I'm working on rewriting tests into new API. Kind regards, Petr
diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh index 3a0fe01..32b4f09 100644 --- a/testcases/lib/tst_net.sh +++ b/testcases/lib/tst_net.sh @@ -19,7 +19,7 @@ # Author: Alexey Kodanev <alexey.kodanev@oracle.com> # -TST_OPTS="6$TST_OPTS" +TST_OPTS="${TST_OPTS}6" TST_PARSE_ARGS_CALLER="$TST_PARSE_ARGS" TST_PARSE_ARGS="tst_net_parse_args" TST_USAGE_CALLER="$TST_USAGE"
If the first character of optstring is set to a colon(tcp_fastopen_run.sh, nfs_lib.sh), getopts should be in silent mode rather than process it as an argument of 6. Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com> --- testcases/lib/tst_net.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)