On Tue, 29 Aug 2023, Maciej Wieczór-Retman wrote:
On 2023-08-23 at 16:15:56 +0300, Ilpo Järvinen wrote:
Benchmark argument is handled by custom argument parsing code which is more complicated than it needs to be.
Process benchmark argument within the normal getopt() handling and drop entirely unnecessary ben_ind and has_ben variables. If -b is not given, setup the default benchmark command right after the switch statement and make -b to goto over it while it terminates the getopt() loop.
Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com Reviewed-by: Reinette Chatre reinette.chatre@intel.com
.../testing/selftests/resctrl/resctrl_tests.c | 71 ++++++++++--------- 1 file changed, 36 insertions(+), 35 deletions(-)
diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c index 94516d1f4307..ae9001ef7b0a 100644 --- a/tools/testing/selftests/resctrl/resctrl_tests.c +++ b/tools/testing/selftests/resctrl/resctrl_tests.c @@ -169,28 +169,35 @@ static void run_cat_test(int cpu_no, int no_of_bits)
int main(int argc, char **argv) {
- bool has_ben = false, mbm_test = true, mba_test = true, cmt_test = true;
- int c, cpu_no = 1, argc_new = argc, i, no_of_bits = 0;
- bool mbm_test = true, mba_test = true, cmt_test = true;
- int c, cpu_no = 1, i, no_of_bits = 0; const char *benchmark_cmd[BENCHMARK_ARGS];
- int ben_ind, tests = 0; char *span_str = NULL; bool cat_test = true; char *skip_reason;
- int tests = 0; int ret;
- for (i = 0; i < argc; i++) {
if (strcmp(argv[i], "-b") == 0) {ben_ind = i + 1;argc_new = ben_ind - 1;has_ben = true;break;}- }
- while ((c = getopt(argc_new, argv, "ht:b:n:p:")) != -1) {
while ((c = getopt(argc, argv, "ht:b:n:p:")) != -1) { char *token;
switch (c) {
case 'b':/** First move optind back to the (first) optarg and* then build the benchmark command using the* remaining arguments.*/optind--;if (argc - optind >= BENCHMARK_ARGS - 1)ksft_exit_fail_msg("Too long benchmark command");Isn't this condition off by two?
I did some testing and the maximum amount of benchmark arguments is 62 while the array of const char* has 64 spaces. Is it supposed to have less than the maximum capacity?
Wouldn't something like this be more valid with BENCHMARK_ARGS equal to 64? : if (argc - optind > BENCHMARK_ARGS)
Certainly not off by two as the array must be NULL terminated but it seems to be off-by-one (to the safe direction), yes.