Re: [PATCH 3/5] perf dso: Use stored fd error instead of stale errno in file_read() and file_size()

On Tue, Aug 11, 2026 at 10:52 AM Arnaldo Carvalho de Melo
<[email protected]> wrote:
>
> From: Arnaldo Carvalho de Melo <[email protected]>
>
> file_read() and file_size() use ret = -errno when
> dso__data(dso)->fd is negative after try_to_open_dso() fails.  By this
> point errno has been through mutex_lock(), nsinfo__mountns_enter(), and
> multiple open() attempts inside try_to_open_dso() — it no longer
> reflects the actual open failure.  If errno happens to be 0, ret = 0
> looks like EOF rather than an error, and file_size() callers like
> dso__data_size() would then report a zero-sized file instead of
> failing.
>
> dso__data(dso)->fd is always negative on failure — -errno from
> __open_dso() when no filename could be built (e.g. -EINVAL, -ENOENT),
> or -1 when do_open() itself failed — and never 0, so use it directly
> instead of reading the stale global errno.
>
> Fixes: 33bdedcea2d7 ("perf tools: Protect dso cache fd with a mutex")
> Reported-by: sashiko-bot <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
> ---
>  tools/perf/util/dso.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 124193453675ca18..32ae5c78cdb906e6 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -1029,7 +1029,8 @@ static ssize_t file_read(struct dso *dso, struct machine *machine,
>
>         if (dso__data(dso)->fd < 0) {
>                 dso__data(dso)->status = DSO_DATA_STATUS_ERROR;
> -               ret = -errno;
> +               /* fd is always negative here: -errno or -1, never 0 */
> +               ret = dso__data(dso)->fd;

nit: we can "assert(ret < 0);" to verify the comment.

>                 goto out;
>         }
>
> @@ -1151,8 +1152,9 @@ static int file_size(struct dso *dso, struct machine *machine)
>         try_to_open_dso(dso, machine);
>
>         if (dso__data(dso)->fd < 0) {
> -               ret = -errno;
>                 dso__data(dso)->status = DSO_DATA_STATUS_ERROR;
> +               /* fd is always negative here: -errno or -1, never 0 */
> +               ret = dso__data(dso)->fd;

nit: we can "assert(ret < 0);" to verify the comment.

Thanks,
Ian

>                 goto out;
>         }
>
> --
> 2.55.0
>