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

On Mon, Aug 10, 2026 at 7:23 AM <[email protected]> wrote:
>
> From: Yuan Chen <[email protected]>
>
> 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. Use ferror() instead, and
> track the too-long-line case explicitly.
>
> Since do_batch() no longer inspects errno to detect read failures, drop
> the USE_LIBCAP errno reset in main() that existed only to keep errno
> clean for the batch mode.
>
> Fixes: 71bb428fe2c1 ("tools: bpf: add bpftool")
> Signed-off-by: Yuan Chen <[email protected]>
> ---
>  tools/bpf/bpftool/main.c | 18 ++++++------------
>  1 file changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
> index c91e1a6e1a1e..0bbb2e198450 100644
> --- a/tools/bpf/bpftool/main.c
> +++ b/tools/bpf/bpftool/main.c
> @@ -336,6 +336,7 @@ static int do_batch(int argc, char **argv)
>         char buf[BATCH_LINE_LEN_MAX], contline[BATCH_LINE_LEN_MAX];
>         char *n_argv[BATCH_ARG_NB_MAX];
>         unsigned int lines = 0;
> +       bool line_too_long = false;

it feels wrong to track this specific error explicitly. we already
have err, so we can check that err is set and then look at errno. or
clear errno before fgets(), some variant of that maybe. but not
explicit bool like what you did here.

I've applied the first patch, but please iteration on this one

>         int n_argc;
>         FILE *fp;
>         char *cp;
> @@ -371,7 +372,7 @@ static int do_batch(int argc, char **argv)
>                         *cp = '\0';
>
>                 if (strlen(buf) == sizeof(buf) - 1) {
> -                       errno = E2BIG;
> +                       line_too_long = true;
>                         break;
>                 }
>
> @@ -429,7 +430,10 @@ static int do_batch(int argc, char **argv)
>                 lines++;
>         }
>
> -       if (errno && errno != ENOENT) {
> +       if (line_too_long) {
> +               p_err("reading batch file failed: %s", strerror(E2BIG));
> +               err = -1;
> +       } else if (ferror(fp)) {
>                 p_err("reading batch file failed: %s", strerror(errno));
>                 err = -1;
>         } else {
> @@ -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;
> --
> 2.54.0
>