Re: [PATCH V3 5/6] tools/perf: Add powerpc HTM auxtrace event processing support

> On 25 Jul 2026, at 12:51 PM, [email protected] wrote:
> 
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] The file `powerpc-htm.c` calls `zalloc()` but fails to include `<linux/zalloc.h>`, resulting in an implicit function declaration.
> - [Low] The file `powerpc-htm.c` embeds `struct auxtrace_queues` and calls its API but fails to directly include `"auxtrace.h"`, relying on brittle implicit inclusion.
> --
> 
> --- Patch [5]: [PATCH V3 5/6] tools/perf: Add powerpc HTM auxtrace event processing support ---
> commit 306ea78646ef6e5fe95dbad9977aff1241d1027a
> Author: Athira Rajeev <[email protected]>
> 
> tools/perf: Add powerpc HTM auxtrace event processing support
> 
> This commit adds the powerpc-htm.c decode stub for powerpc HTM auxtrace.
> It wires the dispatch in perf_event__process_auxtrace_info() to call
> powerpc_htm_process_auxtrace_info().
> 
>> diff --git a/tools/perf/util/powerpc-htm.c b/tools/perf/util/powerpc-htm.c
>> new file mode 100644
>> --- /dev/null
>> +++ b/tools/perf/util/powerpc-htm.c
>> @@ -0,0 +1,116 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <inttypes.h>
>> +#include <fcntl.h>
>> +#include <unistd.h>
>> +#include <string.h>
>> +#include <errno.h>
>> +#include <endian.h>
>> +#include "util/evsel.h"
>> +#include "util/evlist.h"
>> +#include "util/session.h"
>> +#include "color.h"
>> +#include "powerpc-htm.h"
>> +#include "debug.h"
>> +#include "sample.h"
>> +
>> +struct powerpc_htm {
>> + struct auxtrace auxtrace;
>> + struct auxtrace_queues queues;
> 
> [Severity: Low]
> Should this file explicitly include "auxtrace.h"?
> 
> The code embeds struct auxtrace_queues here, and later calls
> auxtrace_queues__init(), both of which are defined in "auxtrace.h". Relying
> on implicit inclusion (for example, through "util/evlist.h") currently works
> but could break the build if intermediate headers are refactored in the
> future.
> 
> [ ... ]
> 
>> +int powerpc_htm_process_auxtrace_info(union perf_event *event,
>> +      struct perf_session *session)
>> +{
>> + struct perf_record_auxtrace_info *auxtrace_info = &event->auxtrace_info;
>> + struct powerpc_htm *htm;
>> + int err;
>> +
>> + if (auxtrace_info->header.size < sizeof(struct perf_record_auxtrace_info) +
>> + HTM_AUXTRACE_PRIV_FIXED)
>> + return -EINVAL;
>> +
>> + htm = zalloc(sizeof(struct powerpc_htm));
> 
> [Severity: High]
> Does this implicit function declaration cause a build failure or pointer
> truncation?
> 
> The function zalloc() is used here, but <linux/zalloc.h> is not included in
> this file. Because the kernel build system uses -Werror, this missing header
> could trigger a build failure.
> 
> If -Werror were disabled, the compiler would assume zalloc() returns a 32-bit
> integer. On 64-bit architectures like PowerPC, this truncates the 64-bit
> pointer. When extended back to 64 bits during assignment, the higher bits are
> lost, which could result in a segmentation fault upon dereference.
> 
> Should <linux/zalloc.h> be included to ensure the return type is properly
> defined?

Sure, V4 will have changes to add zalloc.h and auxtrace.h

Thanks
Athira
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5