On 23/12/2022 02:39, Leo Yan wrote:
On Thu, Dec 22, 2022 at 04:03:21PM +0000, James Clark wrote:
The pattern for accessing EVENT_SOURCE_DEVICE_PATH is duplicated in a few places, so add two utility functions to cover it. Also just use perf_pmu__scan_file() instead of pmu_type() which already does the same thing.
No functional changes.
After I read the article: https://lwn.net/Articles/69419/, good to see this patch uses scnprintf() to replace snprintf().
Signed-off-by: James Clark james.clark@arm.com
tools/perf/arch/arm/util/auxtrace.c | 5 +- tools/perf/arch/x86/util/pmu.c | 12 +-- tools/perf/util/pmu.c | 110 +++++++++++----------------- tools/perf/util/pmu.h | 5 +- 4 files changed, 49 insertions(+), 83 deletions(-)
diff --git a/tools/perf/arch/arm/util/auxtrace.c b/tools/perf/arch/arm/util/auxtrace.c index deeb163999ce..3cb4a6a16112 100644 --- a/tools/perf/arch/arm/util/auxtrace.c +++ b/tools/perf/arch/arm/util/auxtrace.c @@ -55,17 +55,16 @@ static struct perf_pmu **find_all_arm_spe_pmus(int *nr_spes, int *err) static struct perf_pmu **find_all_hisi_ptt_pmus(int *nr_ptts, int *err) {
- const char *sysfs = sysfs__mountpoint(); struct perf_pmu **hisi_ptt_pmus = NULL; struct dirent *dent; char path[PATH_MAX]; DIR *dir = NULL; int idx = 0;
- snprintf(path, PATH_MAX, "%s" EVENT_SOURCE_DEVICE_PATH, sysfs);
- perf_pmu__event_source_devices_scnprintf(path, PATH_MAX);
Nitpick: since 'path' is an array, a good practice is to use 'sizeof(path)' rather than 'PATH_MAX' for passing the length.
Done
[...]
@@ -983,9 +931,14 @@ static struct perf_pmu *pmu_lookup(const char *lookup_name) pmu->cpus = pmu_cpumask(name); pmu->name = strdup(name);
- if (!pmu->name) goto err;
- /* Read type, and ensure that 1 value (type) is successfully assigned */
Maybe I don't understand well, seems to me a better comment is:
/* Read type, and ensure that type value is successfully assigned (return 1) */
Changed it in V3. I was trying to portray that the function returns how many variables were successfully assigned, which is slightly different to 1 == success. But it's probably clearer your way.
The rest looks good to me. With addressing above two comments:
Reviewed-by: Leo Yan leo.yan@linaro.org
Thanks for the review.