Re: [PATCH bpf-next v3 2/3] Update API functions usage to check error

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Tue, Apr 19, 2022 at 9:04 AM grantseltzer <grantseltzer@gmail.com> wrote:
>
> From: Grant Seltzer <grantseltzer@gmail.com>
>
> This updates usage of the following API functions within
> libbpf so their newly added error return is checked:
>
> - bpf_program__set_expected_attach_type()
> - bpf_program__set_type()
>
> Signed-off-by: Grant Seltzer <grantseltzer@gmail.com>
> ---
>  tools/lib/bpf/libbpf.c | 29 +++++++++++++++++++++++------
>  1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 0ed1a8c9c398..7635c50a05c6 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -7005,9 +7005,19 @@ static int bpf_object_init_progs(struct bpf_object *obj, const struct bpf_object
>                         continue;
>                 }
>
> -               bpf_program__set_type(prog, prog->sec_def->prog_type);
> -               bpf_program__set_expected_attach_type(prog, prog->sec_def->expected_attach_type);
> +               err = bpf_program__set_type(prog, prog->sec_def->prog_type);
> +               if (err) {
> +                       pr_warn("prog '%s': failed to initialize: %d, could not set program type\n",
> +                               prog->name, err);
> +                       return err;
> +               }
>
> +               err = bpf_program__set_expected_attach_type(prog, prog->sec_def->expected_attach_type);
> +               if (err) {
> +                       pr_warn("prog '%s': failed to initialize: %d, could not set expected attach type\n",
> +                               prog->name, err);
> +                       return err;
> +               }

this is too paranoid, we know that this will succeed. We can just do:

prog->type = prog->sec_def->prog_type;
prog->expected_attach_type = prog->sec_def->expected_attach_type;

to make this very clear

>  #pragma GCC diagnostic push
>  #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
>                 if (prog->sec_def->prog_type == BPF_PROG_TYPE_TRACING ||
> @@ -8570,8 +8580,7 @@ int bpf_program__set_##NAME(struct bpf_program *prog)             \
>  {                                                              \
>         if (!prog)                                              \
>                 return libbpf_err(-EINVAL);                     \
> -       bpf_program__set_type(prog, TYPE);                      \
> -       return 0;                                               \
> +       return bpf_program__set_type(prog, TYPE);                       \
>  }                                                              \
>                                                                 \
>  bool bpf_program__is_##NAME(const struct bpf_program *prog)    \
> @@ -9678,9 +9687,17 @@ static int bpf_prog_load_xattr2(const struct bpf_prog_load_attr *attr,
>                  * bpf_object__open guessed
>                  */
>                 if (attr->prog_type != BPF_PROG_TYPE_UNSPEC) {
> -                       bpf_program__set_type(prog, attr->prog_type);
> -                       bpf_program__set_expected_attach_type(prog,
> +                       err = bpf_program__set_type(prog, attr->prog_type);
> +                       if (err) {
> +                               pr_warn("could not set program type\n");
> +                               return libbpf_err(err);
> +                       }
> +                       err = bpf_program__set_expected_attach_type(prog,
>                                                               attach_type);
> +                       if (err) {
> +                               pr_warn("could not set expected attach type\n");
> +                               return libbpf_err(err);
> +                       }

same as above, no need to add unnecessary checks and pr_warns


>                 }
>                 if (bpf_program__type(prog) == BPF_PROG_TYPE_UNSPEC) {
>                         /*
> --
> 2.34.1
>