| Message ID | 20210617184216.2075588-2-irogers@google.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [1/4] perf test: Fix non-bash issue with stat bpf counters | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
Em Thu, Jun 17, 2021 at 11:42:14AM -0700, Ian Rogers escreveu: > Having a verbose option will allow shell tests to provide extra failure > details when the fail or skip. Thanks, applied to perf/core. - Arnaldo > Signed-off-by: Ian Rogers <irogers@google.com> > --- > tools/perf/tests/builtin-test.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c > index cbbfe48ab802..a8160b1684de 100644 > --- a/tools/perf/tests/builtin-test.c > +++ b/tools/perf/tests/builtin-test.c > @@ -577,11 +577,14 @@ struct shell_test { > static int shell_test__run(struct test *test, int subdir __maybe_unused) > { > int err; > - char script[PATH_MAX]; > + char script[PATH_MAX + 3]; > struct shell_test *st = test->priv; > > path__join(script, sizeof(script), st->dir, st->file); > > + if (verbose) > + strncat(script, " -v", sizeof(script)); > + > err = system(script); > if (!err) > return TEST_OK; > -- > 2.32.0.288.g62a8d224e6-goog >
Em Thu, Jun 17, 2021 at 04:19:06PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Thu, Jun 17, 2021 at 11:42:14AM -0700, Ian Rogers escreveu: > > Having a verbose option will allow shell tests to provide extra failure > > details when the fail or skip. > > > Thanks, applied to perf/core. > > - Arnaldo > > > Signed-off-by: Ian Rogers <irogers@google.com> > > --- > > tools/perf/tests/builtin-test.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c > > index cbbfe48ab802..a8160b1684de 100644 > > --- a/tools/perf/tests/builtin-test.c > > +++ b/tools/perf/tests/builtin-test.c > > @@ -577,11 +577,14 @@ struct shell_test { > > static int shell_test__run(struct test *test, int subdir __maybe_unused) > > { > > int err; > > - char script[PATH_MAX]; > > + char script[PATH_MAX + 3]; > > struct shell_test *st = test->priv; > > > > path__join(script, sizeof(script), st->dir, st->file); probably you need to add a '- 3' after the sizeof above, right? > > > > + if (verbose) > > + strncat(script, " -v", sizeof(script)); > > + Seemed simple enough, but gcc knows better, I'm removing this one: tests/builtin-test.c:586:26: error: the value of the size argument in 'strncat' is too large, might lead to a buffer overflow [-Werror,-Wstrncat-size] strncat(script, " -v", sizeof(script)); ^~~~~~~~~~~~~~ tests/builtin-test.c:586:26: note: change the argument to be the free space in the destination buffer minus the terminating null byte strncat(script, " -v", sizeof(script)); ^~~~~~~~~~~~~~ sizeof(script) - strlen(script) - 1 1 error generated. make[3]: *** [/git/perf-5.13.0-rc4/tools/build/Makefile.build:139: tests] Error 2 77 31.98 ubuntu:21.04 : FAIL gcc version 10.3.0 (Ubuntu 10.3.0-1ubuntu1) tests/builtin-test.c:586:26: error: the value of the size argument in 'strncat' is too large, might lead to a buffer overflow [-Werror,-Wstrncat-size] strncat(script, " -v", sizeof(script)); ^~~~~~~~~~~~~~ tests/builtin-test.c:586:26: note: change the argument to be the free space in the destination buffer minus the terminating null byte strncat(script, " -v", sizeof(script)); ^~~~~~~~~~~~~~ sizeof(script) - strlen(script) - 1 1 error generated. make[3]: *** [/git/perf-5.13.0-rc4/tools/build/Makefile.build:139: tests] Error 2 > > err = system(script); > > if (!err) > > return TEST_OK; > > -- > > 2.32.0.288.g62a8d224e6-goog > > > > -- > > - Arnaldo
On Fri, Jun 18, 2021 at 9:49 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > Em Thu, Jun 17, 2021 at 04:19:06PM -0300, Arnaldo Carvalho de Melo escreveu: > > Em Thu, Jun 17, 2021 at 11:42:14AM -0700, Ian Rogers escreveu: > > > Having a verbose option will allow shell tests to provide extra failure > > > details when the fail or skip. > > > > > > Thanks, applied to perf/core. > > > > - Arnaldo > > > > > Signed-off-by: Ian Rogers <irogers@google.com> > > > --- > > > tools/perf/tests/builtin-test.c | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c > > > index cbbfe48ab802..a8160b1684de 100644 > > > --- a/tools/perf/tests/builtin-test.c > > > +++ b/tools/perf/tests/builtin-test.c > > > @@ -577,11 +577,14 @@ struct shell_test { > > > static int shell_test__run(struct test *test, int subdir __maybe_unused) > > > { > > > int err; > > > - char script[PATH_MAX]; > > > + char script[PATH_MAX + 3]; > > > struct shell_test *st = test->priv; > > > > > > path__join(script, sizeof(script), st->dir, st->file); > > probably you need to add a '- 3' after the sizeof above, right? Either way is fine, but -3 is ok with me. > > > > > > + if (verbose) > > > + strncat(script, " -v", sizeof(script)); > > > + > > Seemed simple enough, but gcc knows better, I'm removing this one: > > tests/builtin-test.c:586:26: error: the value of the size argument in 'strncat' is too large, might lead to a buffer overflow [-Werror,-Wstrncat-size] > strncat(script, " -v", sizeof(script)); > ^~~~~~~~~~~~~~ > tests/builtin-test.c:586:26: note: change the argument to be the free space in the destination buffer minus the terminating null byte > strncat(script, " -v", sizeof(script)); > ^~~~~~~~~~~~~~ > sizeof(script) - strlen(script) - 1 > 1 error generated. > make[3]: *** [/git/perf-5.13.0-rc4/tools/build/Makefile.build:139: tests] Error 2 > 77 31.98 ubuntu:21.04 : FAIL gcc version 10.3.0 (Ubuntu 10.3.0-1ubuntu1) > tests/builtin-test.c:586:26: error: the value of the size argument in 'strncat' is too large, might lead to a buffer overflow [-Werror,-Wstrncat-size] > strncat(script, " -v", sizeof(script)); > ^~~~~~~~~~~~~~ > tests/builtin-test.c:586:26: note: change the argument to be the free space in the destination buffer minus the terminating null byte > strncat(script, " -v", sizeof(script)); > ^~~~~~~~~~~~~~ > sizeof(script) - strlen(script) - 1 > 1 error generated. > make[3]: *** [/git/perf-5.13.0-rc4/tools/build/Makefile.build:139: tests] Error 2 Thanks gcc :-) Do you want me to resend the patch? Ian > > > err = system(script); > > > if (!err) > > > return TEST_OK; > > > -- > > > 2.32.0.288.g62a8d224e6-goog > > > > > > > -- > > > > - Arnaldo > > -- > > - Arnaldo
Em Fri, Jun 18, 2021 at 09:45:05PM -0700, Ian Rogers escreveu: > On Fri, Jun 18, 2021 at 9:49 AM Arnaldo Carvalho de Melo > <acme@kernel.org> wrote: > > > > Em Thu, Jun 17, 2021 at 04:19:06PM -0300, Arnaldo Carvalho de Melo escreveu: > > > Em Thu, Jun 17, 2021 at 11:42:14AM -0700, Ian Rogers escreveu: > > > > Having a verbose option will allow shell tests to provide extra failure > > > > details when the fail or skip. > > > > > > > > > Thanks, applied to perf/core. > > > > > > - Arnaldo > > > > > > > Signed-off-by: Ian Rogers <irogers@google.com> > > > > --- > > > > tools/perf/tests/builtin-test.c | 5 ++++- > > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c > > > > index cbbfe48ab802..a8160b1684de 100644 > > > > --- a/tools/perf/tests/builtin-test.c > > > > +++ b/tools/perf/tests/builtin-test.c > > > > @@ -577,11 +577,14 @@ struct shell_test { > > > > static int shell_test__run(struct test *test, int subdir __maybe_unused) > > > > { > > > > int err; > > > > - char script[PATH_MAX]; > > > > + char script[PATH_MAX + 3]; > > > > struct shell_test *st = test->priv; > > > > > > > > path__join(script, sizeof(script), st->dir, st->file); > > > > probably you need to add a '- 3' after the sizeof above, right? > > Either way is fine, but -3 is ok with me. > > > > > > > > > + if (verbose) > > > > + strncat(script, " -v", sizeof(script)); > > > > + > > > > Seemed simple enough, but gcc knows better, I'm removing this one: > > > > tests/builtin-test.c:586:26: error: the value of the size argument in 'strncat' is too large, might lead to a buffer overflow [-Werror,-Wstrncat-size] > > strncat(script, " -v", sizeof(script)); > > ^~~~~~~~~~~~~~ > > tests/builtin-test.c:586:26: note: change the argument to be the free space in the destination buffer minus the terminating null byte > > strncat(script, " -v", sizeof(script)); > > ^~~~~~~~~~~~~~ > > sizeof(script) - strlen(script) - 1 > > 1 error generated. > > make[3]: *** [/git/perf-5.13.0-rc4/tools/build/Makefile.build:139: tests] Error 2 > > 77 31.98 ubuntu:21.04 : FAIL gcc version 10.3.0 (Ubuntu 10.3.0-1ubuntu1) > > tests/builtin-test.c:586:26: error: the value of the size argument in 'strncat' is too large, might lead to a buffer overflow [-Werror,-Wstrncat-size] > > strncat(script, " -v", sizeof(script)); > > ^~~~~~~~~~~~~~ > > tests/builtin-test.c:586:26: note: change the argument to be the free space in the destination buffer minus the terminating null byte > > strncat(script, " -v", sizeof(script)); > > ^~~~~~~~~~~~~~ > > sizeof(script) - strlen(script) - 1 > > 1 error generated. > > make[3]: *** [/git/perf-5.13.0-rc4/tools/build/Makefile.build:139: tests] Error 2 > > Thanks gcc :-) Do you want me to resend the patch? In such cases please do, - Arnaldo
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c index cbbfe48ab802..a8160b1684de 100644 --- a/tools/perf/tests/builtin-test.c +++ b/tools/perf/tests/builtin-test.c @@ -577,11 +577,14 @@ struct shell_test { static int shell_test__run(struct test *test, int subdir __maybe_unused) { int err; - char script[PATH_MAX]; + char script[PATH_MAX + 3]; struct shell_test *st = test->priv; path__join(script, sizeof(script), st->dir, st->file); + if (verbose) + strncat(script, " -v", sizeof(script)); + err = system(script); if (!err) return TEST_OK;
Having a verbose option will allow shell tests to provide extra failure details when the fail or skip. Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/tests/builtin-test.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)