linux-kernel - Re: [PATCH 1/1] perf tests record: Fail the test if the errs counter is not zero

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAP-5=fUbxhcATmqWBU8fDeuLB7qWhnrZH=kBP=pVBUrw_wTXgA@mail.gmail.com>
Date:   Wed, 28 Sep 2022 09:41:16 -0700
From:   Ian Rogers <irogers@...gle.com>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>
Cc:     Adrian Hunter <adrian.hunter@...el.com>,
        Jiri Olsa <jolsa@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1] perf tests record: Fail the test if the errs counter
 is not zero

On Wed, Sep 28, 2022 at 9:22 AM Arnaldo Carvalho de Melo
<acme@...nel.org> wrote:
>
> We were just checking for the 'err' variable, when we should really see
> if there was some of the many checked errors that don't stop the test
> right away.
>
> Detected with clang 15.0.0:
>
>   44    75.23 fedora:37                     : FAIL clang version 15.0.0 (Fedora 15.0.0-2.fc37)
>
>     tests/perf-record.c:68:16: error: variable 'errs' set but not used [-Werror,-Wunused-but-set-variable]
>             int err = -1, errs = 0, i, wakeups = 0;
>                           ^
>     1 error generated.
>
> The patch introducing this 'perf test' entry had that check:
>
>   +       return (err < 0 || errs > 0) ? -1 : 0;
>
> But at some point we lost that:
>
>   -       return (err < 0 || errs > 0) ? -1 : 0;
>   +       if (err == -EACCES)
>   +               return TEST_SKIP;
>   +       if (err < 0)
>   +               return TEST_FAIL;
>   +       return TEST_OK
>
> Put it back.
>
> Fixes: 2cf88f4614c996e5 ("perf test: Use skip in PERF_RECORD_*")
> Cc: Adrian Hunter <adrian.hunter@...el.com>
> Cc: Ian Rogers <irogers@...gle.com>
> Cc: Jiri Olsa <jolsa@...nel.org>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>

Apologies for that.

Acked-by: Ian Rogers <irogers@...gle.com>

Thanks,
Ian

> ---
>  tools/perf/tests/perf-record.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/tests/perf-record.c b/tools/perf/tests/perf-record.c
> index 6a001fcfed68e517..4952abe716f318b0 100644
> --- a/tools/perf/tests/perf-record.c
> +++ b/tools/perf/tests/perf-record.c
> @@ -332,7 +332,7 @@ static int test__PERF_RECORD(struct test_suite *test __maybe_unused, int subtest
>  out:
>         if (err == -EACCES)
>                 return TEST_SKIP;
> -       if (err < 0)
> +       if (err < 0 || errs != 0)
>                 return TEST_FAIL;
>         return TEST_OK;
>  }
> --
> 2.37.3
>
>
> ----- End forwarded message -----
>
> --
>
> - Arnaldo