Re: [PATCH v5 14/58] perf python: Add wrapper for perf_data file abstraction

sashiko-bot
2026-04-24T17:35:02Z

Sashiko AI review found 1 potential issue(s):
- [Medium] The Python `perf.data` API silently ignores the user-provided `fd` argument and opens a local `perf.data` file instead.
--

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

perf python: Add wrapper for perf_data file abstraction

This commit adds a Python wrapper for the perf_data file abstraction.
It allows users to open perf data files by path or file descriptor
to support sessions.

> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 0424290f8b77f8..a2cdd92e05482b 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
[ ... ]
> @@ -2296,6 +2297,92 @@ static PyObject *pyrf__metrics(PyObject *self, PyObject *args)
> return list;
> }
>
> +struct pyrf_data {
> + PyObject_HEAD
> +
> + struct perf_data data;
> +};
> +
> +static int pyrf_data__init(struct pyrf_data *pdata, PyObject *args, PyObject *kwargs)
> +{
> + static char *kwlist[] = { "path", "fd", NULL };
> + char *path = NULL;
> + int fd = -1;
> +
> + if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|si", kwlist, &path, &fd))
> + return -1;
> +
> + if (pdata->data.open)
> + perf_data__close(&pdata->data);
> + free((char *)pdata->data.path);
> +
> + if (!path)
> + path = "perf.data";
> +
> + pdata->data.path = strdup(path);
> + if (!pdata->data.path) {
> + PyErr_NoMemory();
> + return -1;
> + }
> +
> + if (fd != -1) {
> + struct stat st;
> +
> + if (fstat(fd, &st) < 0 || !S_ISFIFO(st.st_mode)) {
> + PyErr_SetString(PyExc_ValueError,
> + "fd argument is only supported for pipes");
> + free((char *)pdata->data.path);
> + pdata->data.path = NULL;
> + return -1;
> + }
> + }
> +
> + pdata->data.mode = PERF_DATA_MODE_READ;
> + pdata->data.file.fd = fd;
> + if (perf_data__open(&pdata->data) < 0) {

If an fd is provided but path is omitted, path defaults to "perf.data".
Does this cause perf_data__open() to ignore the provided fd?

Looking at perf_data__open(), it calls check_pipe() which evaluates
!strcmp(data->path, "-"). Since the path is "perf.data", check_pipe() returns
false, and perf_data__open() opens the local "perf.data" file instead,
overwriting the validated pipe fd unless the caller explicitly passed path="-".

> + PyErr_Format(PyExc_IOError, "Failed to open perf data: %s",
> + pdata->data.path ? pdata->data.path : "perf.data");
> + return -1;
> + }
> + return 0;
> +}

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