Re: [PATCH v3 1/6] perf kvm: Factor out kvm_need_default_arch_event()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The file tools/perf/util/kvm-stat.c introduces usage of calloc and free but fails to explicitly include <stdlib.h>, violating POSIX/musl compatibility guidelines.
--

commit 706198153306324cdd16cca21dab43f87d56e722
Author: Namhyung Kim <namhyung@kernel.org>

perf kvm: Factor out kvm_need_default_arch_event()

This commit extracts the common logic for checking if an event command line
option exists into a new generic function kvm_need_default_arch_event().
This avoids duplicating the checking logic in each architecture's
kvm_add_default_arch_event() function.

> diff --git a/tools/perf/util/kvm-stat.c b/tools/perf/util/kvm-stat.c
> index 755ab659a05c3..c571d2ed711a8 100644
> --- a/tools/perf/util/kvm-stat.c
> +++ b/tools/perf/util/kvm-stat.c
> @@ -1,8 +1,10 @@
>  // SPDX-License-Identifier: GPL-2.0
>  #include "debug.h"
> +#include "env.h"
>  #include "evsel.h"
>  #include "kvm-stat.h"
>  #include <dwarf-regs.h>
> +#include <subcmd/parse-options.h>
>  
>  bool kvm_exit_event(struct evsel *evsel)
>  {

[ ... ]

> @@ -268,3 +270,42 @@ int kvm_add_default_arch_event(uint16_t e_machine, int *argc, const char **argv)
>  		return 0;
>  	}
>  }
> +
> +bool kvm_need_default_arch_event(uint16_t e_machine, int argc, const char **argv)
> +{
> +	const char **tmp_argv;
> +	bool event = false;
> +	int i;
> +
> +	const struct option event_options[] = {
> +		OPT_BOOLEAN('e', "event", &event, NULL),
> +		OPT_BOOLEAN(0, "pfm-events", &event, NULL),
> +		OPT_END()
> +	};
> +
> +	switch (e_machine) {
> +	case EM_PPC:
> +	case EM_PPC64:
> +		break;
> +	case EM_X86_64:
> +	case EM_386:
> +		if (!x86__is_intel_cpu())
> +			return false;
> +		break;
> +	default:
> +		return false;
> +	}
> +
> +	/* parse_options() may change the argv, let's make a copy */
> +	tmp_argv = calloc(argc + 1, sizeof(char *));

[Severity: Low]
Does this file need an explicit include of <stdlib.h>?

It looks like calloc() and free() are used here, but <stdlib.h> is not directly 
included in tools/perf/util/kvm-stat.c. Relying on implicit includes could cause 
build failures under musl libc where headers are strictly separated.

> +	if (!tmp_argv)
> +		return false;
> +
> +	for (i = 0; i < argc; i++)
> +		tmp_argv[i] = argv[i];
> +
> +	parse_options(argc, tmp_argv, event_options, NULL, PARSE_OPT_KEEP_UNKNOWN);
> +	free(tmp_argv);
> +
> +	return !event;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701194153.401218-1-namhyung@kernel.org?part=1