Re: [PATCH v3] perf data: Clean up use_stdio and structures

sashiko-bot
2026-04-08T07:06:32Z

Sashiko AI review found 2 potential issue(s):
- [High] The newly introduced `perf_data_file__seek` API silently truncates 64-bit file offsets on 32-bit platforms.
- [Medium] Missed API conversion in `record__finish_output` bypasses the new stdio abstraction.
--

commit f45ea505995a4b5ea83270dc73763efa7e6f535d
Author: Ian Rogers <irogers@google.com>

perf data: Clean up use_stdio and structures

This commit reorganizes perf_data_file to better abstract use_stdio,
adding new accessors like perf_data_file__seek() to ensure stdio buffering
is respected when a file pointer is used instead of a file descriptor.

> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 60d7640683024..88c2e4ea5c2d7 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
[ ... ]
> @@ -1847,8 +1847,10 @@ record__finish_output(struct record *rec)
> rec->session->header.data_size += rec->bytes_written;
> data->file.size = lseek(perf_data__fd(data), 0, SEEK_CUR);

Should this lseek() also be converted to use perf_data__seek()?

If use_stdio is enabled here, perf_data__fd() will extract the raw file
descriptor and lseek() will bypass the stdio buffering. This seems to
miss the abstraction introduced in this patch.

> if (record__threads_enabled(rec)) {
> - for (i = 0; i < data->dir.nr; i++)
> - data->dir.files[i].size = lseek(data->dir.files[i].fd, 0, SEEK_CUR);
> + for (i = 0; i < data->dir.nr; i++) {
> + data->dir.files[i].size =
> + perf_data_file__seek(&data->dir.files[i], 0, SEEK_CUR);
> + }
> }

[ ... ]

> diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
> index 90df41da1a32b..a1914d71e3331 100644
> --- a/tools/perf/util/data.c
> +++ b/tools/perf/util/data.c
[ ... ]
> ssize_t perf_data__write(struct perf_data *data,
> void *buf, size_t size)
> {
> - if (data->use_stdio) {
> - if (fwrite(buf, size, 1, data->file.fptr) == 1)
> - return size;
> - return -1;
> - }
> return perf_data_file__write(&data->file, buf, size);
> }
>
> +off_t perf_data_file__seek(struct perf_data_file *file, off_t offset, int whence)
> +{
> + if (file->use_stdio) {
> + off_t res = fseek(file->fptr, offset, whence);
> +
> + return res < 0 ? -1 : ftell(file->fptr);

Does this silently truncate 64-bit file offsets on 32-bit platforms?

Both fseek() and ftell() operate on the long data type, which is 32 bits on
32-bit architectures. Since the offset parameter and return type are off_t
(which is 64-bit when _FILE_OFFSET_BITS=64 is used), shouldn't fseeko() and
ftello() be used here instead to correctly support large files?

> + }
> + return lseek(file->fd, offset, whence);
> +}

--
Sashiko AI review · https://sashiko.dev/#/patchset/20260408064721.138338-1-irogers@google.com?part=1