Re: [PATCH perf-tools-next v5 1/5] perf ftrace: Optimise __cmd_ftrace() stack frame with strbuf streaming

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] perf: missing explicit inclusion of <stdio.h> for POSIX stdio streams
--

commit 84dafb5e4aabd0b60e918d9f85e26ec8e27d5c88
Author: Aaron Tomlin <atomlin@atomlin.com>

perf ftrace: Optimise __cmd_ftrace() stack frame with strbuf streaming

This commit optimizes __cmd_ftrace() by replacing a fixed-size stack buffer
with heap allocation and a dynamic strbuf for trace data accumulation. It
also introduces error checking and stream flushing to improve robustness
during steady-state polling.

> diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> index 6017493ff1797..6ee72ae90f67a 100644
> --- a/tools/perf/builtin-ftrace.c
> +++ b/tools/perf/builtin-ftrace.c
> @@ -41,7 +41,10 @@
>  #include "util/units.h"
>  #include "util/parse-sublevel-options.h"
>  
> +#include "util/strbuf.h"
> +

[Severity: Low]
Does this file explicitly include <stdio.h>?

This isn't a runtime bug, but relying on implicit inclusion via other headers
for POSIX stdio streams and functions (like stdout, fprintf, fflush, and
ferror) violates the perf subsystem guidelines for musl compatibility.

Explicitly including <stdio.h> ensures the code doesn't cause build breakages
on libc implementations that strictly separate declarations, like musl.

[ ... ]
> @@ -806,24 +818,42 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace)
>  	}
>  
>  	while (!done) {
> -		if (poll(&pollfd, 1, -1) < 0)
> +		if (poll(&pollfd, 1, -1) < 0) {
> +			if (errno == EINTR)
> +				continue;
>  			break;
> +		}
>  
>  		if (pollfd.revents & POLLIN) {
> -			int n = read(trace_fd, buf, sizeof(buf));
> -			if (n < 0)
> +			int n = read(trace_fd, buf, TRACE_BUF_SIZE);
> +
> +			if (n < 0) {
> +				if (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR)
> +					continue;
>  				break;
> -			if (fwrite(buf, n, 1, stdout) != 1)
> +			}
> +			if (n == 0)
>  				break;
> +			for (int i = 0; i < n; i++) {
> +				if (buf[i] == '\n') {
> +					if (fprintf(stdout, "%s\n", linebuf.buf) < 0)
> +						break;
> +					strbuf_setlen(&linebuf, 0);
> +				} else {
> +					if (strbuf_addch(&linebuf, buf[i]) < 0)
> +						goto out_close_fd;
> +				}
> +			}
>  			/* flush output since stdout is in full buffering mode due to pager */
> -			fflush(stdout);
> +			if (fflush(stdout) < 0 || ferror(stdout))
> +				break;
>  		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015011155.414667-1-atomlin@atomlin.com?part=1