Re: [PATCH v1 09/58] perf data: Add open flag

sashiko-bot
2026-04-20T00:44:20Z

Sashiko AI review found 2 potential issue(s):
- [Medium] File rotation via `perf_data__switch()` is broken because `data->open` is not reset when the underlying file is closed.
- [Medium] Memory leak of `data->file.path` in `open_dir()` when directory creation fails.
--

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

perf data: Add open flag

This commit adds an open flag to the perf_data struct to avoid double opens
and ensure only open files are closed. This helps with python integration
where data files are opened before being passed to a session.

> diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
> index 94dc534a73866..9a7195c68a16d 100644
> --- a/tools/perf/util/data.c
> +++ b/tools/perf/util/data.c
> @@ -360,9 +360,16 @@ static int open_dir(struct perf_data *data)
>
> int perf_data__open(struct perf_data *data)
> {
> - if (check_pipe(data))
> + int ret;
> +
> + if (data->open)
> return 0;

Does this break file rotation via perf_data__switch()?

When perf record --switch-output triggers a file switch, it calls
perf_data__switch() which closes the current file via
perf_data_file__close(&data->file). This sets data->file.fd = -1
but does not reset the newly added data->open flag to false.

When perf_data__switch() subsequently attempts to open the new file by
calling perf_data__open(data), it hits this early exit and returns
immediately.

This causes subsequent calls like perf_data__seek() to attempt operations
on a -1 file descriptor, aborting the recording session.

[ ... ]

> @@ -375,16 +382,24 @@ int perf_data__open(struct perf_data *data)
>
> void perf_data__close(struct perf_data *data)
> {
> + if (!data->open)
> + return;
> +

Does this cause a memory leak of data->file.path if directory
creation fails?

In open_dir(), data->file.path is allocated via asprintf(). If the
subsequent mkdir() call fails, open_dir() returns -1 immediately.
Because perf_data__open() receives an error, it returns without setting
data->open = true.

When the caller handles the error by calling perf_data__close() to
clean up, it hits this new early return and bypasses the call to
perf_data_file__close(), permanently leaking the allocated string.

> if (perf_data__is_dir(data))
> perf_data__close_dir(data);
>
> perf_data_file__close(&data->file);
> + data->open = false;
> }

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