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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing explicit inclusion of `<assert.h>` for the newly added `assert()` calls, violating musl libc compatibility guidelines.
--

--- Patch [3]: [PATCH 3/5] perf dso: Use stored fd error instead of stale errno in file_read() and file_size() ---
commit 9359e4a3d3517acb5c94e232e0f7e3c94b3a7824
Author: Arnaldo Carvalho de Melo <[email protected]>

perf dso: Use stored fd error instead of stale errno in file_read() and file_size()

This commit updates file_read() and file_size() to use the negative fd value
as the error code directly instead of relying on errno. This prevents issues 
where a stale errno could incorrectly appear as an EOF rather than an open 
failure.

> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 124193453675c..c7fb9e1d07f14 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -1029,7 +1029,9 @@ 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;
> +		assert(ret < 0);

[Severity: Low]
Does this missing include break musl libc compatibility?

The perf subsystem guidelines require explicit inclusion of POSIX standard
headers for any libc macros used. Could you add the <assert.h> header at the
top of tools/perf/util/dso.c to ensure this compiles properly under musl?

>  		goto out;
>  	}

[ ... ]

> @@ -1151,8 +1153,10 @@ 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;
> +		assert(ret < 0);

[Severity: Low]
Is it possible to ensure <assert.h> is included for this usage as well? 

>  		goto out;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3