Re: [PATCH] perf test pmu: Set uninitialized PMU alias to null

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Thu, Aug 08, 2024 at 12:37:49PM +0200, vmolnaro@redhat.com wrote:
> From: Veronika Molnarova <vmolnaro@redhat.com>
> 
> Commit 3e0bf9 ("perf pmu: Restore full PMU name wildcard support") adds
> a test case "PMU cmdline match" that covers PMU name wildcard support
> provided by function perf_pmu__match(). The test works with a wide
> range of supported combinations of PMU name matching but omits the case
> that if the perf_pmu__match() cannot match the PMU name to the wildcard,
> it tries to match its alias. However, this variable is not set up,
> causing the test case to fail when run with subprocesses or to segfault
> if run as a single process.
> 
> ./perf test -vv 9
>   9: Sysfs PMU tests                                                 :
>   9.1: Parsing with PMU format directory                             : Ok
>   9.2: Parsing with PMU event                                        : Ok
>   9.3: PMU event names                                               : Ok
>   9.4: PMU name combining                                            : Ok
>   9.5: PMU name comparison                                           : Ok
>   9.6: PMU cmdline match                                             : FAILED!
> 
> ./perf test -F 9
>   9.1: Parsing with PMU format directory                             : Ok
>   9.2: Parsing with PMU event                                        : Ok
>   9.3: PMU event names                                               : Ok
>   9.4: PMU name combining                                            : Ok
>   9.5: PMU name comparison                                           : Ok
> Segmentation fault (core dumped)
> 
> Initialize the PMU alias to null for all tests of perf_pmu__match()
> as this functionality is not being tested and the alias matching works
> exactly the same as the matching of the PMU name.
> 
> ./perf test -F 9
>   9.1: Parsing with PMU format directory                             : Ok
>   9.2: Parsing with PMU event                                        : Ok
>   9.3: PMU event names                                               : Ok
>   9.4: PMU name combining                                            : Ok
>   9.5: PMU name comparison                                           : Ok
>   9.6: PMU cmdline match                                             : Ok
> 
> Fixes: 3e0bf9 ("perf pmu: Restore full PMU name wildcard support")
> Signed-off-by: Veronika Molnarova <vmolnaro@redhat.com>
> ---
>  tools/perf/tests/pmu.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/tests/pmu.c b/tools/perf/tests/pmu.c
> index 0b2f04a55d7b..a4730b5dc0d9 100644
> --- a/tools/perf/tests/pmu.c
> +++ b/tools/perf/tests/pmu.c
> @@ -453,11 +453,13 @@ static int test__name_cmp(struct test_suite *test __maybe_unused, int subtest __
>  /**
>   * Test perf_pmu__match() that's used to search for a PMU given a name passed
>   * on the command line. The name that's passed may also be a filename type glob
> - * match.
> + * match. If the name does not match, perf_pmu__match() attempts to match the
> + * alias of the PMU, if provided.
>   */
>  static int test__pmu_match(struct test_suite *test __maybe_unused, int subtest __maybe_unused)
>  {
>  	struct perf_pmu test_pmu;
> +	test_pmu.alias_name = NULL;

We can do a bit more future proofing by instead doing:

	struct perf_pmu test_pmu = {
		.name = "pmuname",
	};

So that all the other fields are initialized to zero, ok? I'll do this
change and make a note in the commit, lemme know if you disagree.

- Arnaldo
  
>  	test_pmu.name = "pmuname";
>  	TEST_ASSERT_EQUAL("Exact match", perf_pmu__match(&test_pmu, "pmuname"),	     true);
> -- 
> 2.43.0
>