Re: [PATCH bpf-next v6 1/2] bpftool: fix spurious batch file read error

> diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
> index c91e1a6e1a1e..7a0c214f08a0 100644
> --- a/tools/bpf/bpftool/main.c
> +++ b/tools/bpf/bpftool/main.c
> @@ -365,7 +365,11 @@ static int do_batch(int argc, char **argv)
>
>  	if (json_output)
>  		jsonw_start_array(json_wtr);
> -	while (fgets(buf, sizeof(buf), fp)) {
> +	for (;;) {
> +		errno = 0;
> +		if (!fgets(buf, sizeof(buf), fp))
> +			break;
> +

[ ... ]

>  		if (errno && errno != ENOENT) {
>  			p_err("reading batch file failed: %s", strerror(errno));
>  			err = -1;

This isn't a bug, but now that errno is cleared before each fgets() and the
post-loop check only sees the last fgets() outcome, is the errno != ENOENT
exemption still needed?

Looking at the code, fgets() and read() cannot fail with ENOENT. The
exemption was load-bearing before this change: commands like map show
(map.c:740), link show (link.c:1356), and map dump (map.c:875) all return
with errno == ENOENT, and the exemption prevented those stale values from
triggering false failures. This patch fixes that at the source by clearing
errno before each read.

Would dropping the errno != ENOENT check here (similar to how the
USE_LIBCAP reset is removed) make the intent clearer?

>
> @@ -467,16 +471,6 @@ int main(int argc, char **argv)
>
>  	setlinebuf(stdout);
>
> -#ifdef USE_LIBCAP
> -	/* Libcap < 2.63 hooks before main() to compute the number of
> -	 * capabilities of the running kernel, and doing so it calls prctl()
> -	 * which may fail and set errno to non-zero.
> -	 * Let's reset errno to make sure this does not interfere with the
> -	 * batch mode.
> -	 */
> -	errno = 0;
> -#endif
> -
>  	last_do_help = do_help;
>  	pretty_output = false;
>  	json_output = false;

This isn't a bug, but could the changelog use an example that still
reproduces on current bpf-next?

The commit message cites "map dump's EBADF from a double close" as the
example of a stale errno:

>     do_batch() checks errno after the read loop to detect read failures,
>     but fgets() does not clear errno on success, so a stale errno left by
>     a previously executed command (e.g. map dump's EBADF from a double
>     close) makes bpftool report a batch file read failure and exit with an
>     error even though every command succeeded.

Looking at the current code, map_dump() no longer closes the fd
(tools/bpf/bpftool/map.c:893-898 just has exit_free: free(key);
free(value); free_map_kv_btf(btf);), and do_dump() is the sole owner
(map.c:941-962). So there's no double close and no EBADF left behind.

A successful map dump actually leaves errno == ENOENT from
bpf_map_get_next_key() exhaustion (map.c:874-877), which the existing
errno != ENOENT exemption already suppressed. This means map dump isn't
actually a command that could have triggered the symptom described.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32712601990