On Wed, Jul 21, 2021 at 8:38 AM Quentin Monnet <quentin@xxxxxxxxxxxxx> wrote: > > Rename function btf__get_from_id() as btf__load_from_kernel_by_id() to > better indicate what the function does. Change the new function so that, > instead of requiring a pointer to the pointer to update and returning > with an error code, it takes a single argument (the id of the BTF > object) and returns the corresponding pointer. This is more in line with > the existing constructors. > > The other tools calling the deprecated btf__get_from_id() function will > be updated in a future commit. > > References: > > - https://github.com/libbpf/libbpf/issues/278 > - https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#btfh-apis > > v2: > - Instead of a simple renaming, change the new function to make it > return the pointer to the btf struct. > - API v0.5.0 instead of v0.6.0. We generally keep such version changes to cover letters. It keeps each individual commit clean and collects full history in the cover letter which becomes a body of merge commit when the whole patch set is applied. For next revision please consolidate the history in the cover letter. Thanks! > > Signed-off-by: Quentin Monnet <quentin@xxxxxxxxxxxxx> > Acked-by: John Fastabend <john.fastabend@xxxxxxxxx> > --- > tools/lib/bpf/btf.c | 25 +++++++++++++++++-------- > tools/lib/bpf/btf.h | 1 + > tools/lib/bpf/libbpf.c | 5 +++-- > tools/lib/bpf/libbpf.map | 1 + > 4 files changed, 22 insertions(+), 10 deletions(-) > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c > index 7e0de560490e..6654bdee7ad7 100644 > --- a/tools/lib/bpf/btf.c > +++ b/tools/lib/bpf/btf.c > @@ -1383,21 +1383,30 @@ struct btf *btf_get_from_fd(int btf_fd, struct btf *base_btf) > return btf; > } > > +struct btf *btf__load_from_kernel_by_id(__u32 id) > +{ > + struct btf *btf; > + int btf_fd; > + > + btf_fd = bpf_btf_get_fd_by_id(id); > + if (btf_fd < 0) > + return ERR_PTR(-errno); please use libbpf_err_ptr() for consistency, see bpf_object__open_mem() for an example > + > + btf = btf_get_from_fd(btf_fd, NULL); > + close(btf_fd); > + > + return libbpf_ptr(btf); > +} > + > int btf__get_from_id(__u32 id, struct btf **btf) > { > struct btf *res; > - int err, btf_fd; > + int err; > > *btf = NULL; > - btf_fd = bpf_btf_get_fd_by_id(id); > - if (btf_fd < 0) > - return libbpf_err(-errno); > - > - res = btf_get_from_fd(btf_fd, NULL); > + res = btf__load_from_kernel_by_id(id); > err = libbpf_get_error(res); > > - close(btf_fd); > - > if (err) > return libbpf_err(err); > > diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h > index fd8a21d936ef..3db9446bc133 100644 > --- a/tools/lib/bpf/btf.h > +++ b/tools/lib/bpf/btf.h > @@ -68,6 +68,7 @@ LIBBPF_API const void *btf__get_raw_data(const struct btf *btf, __u32 *size); > LIBBPF_API const char *btf__name_by_offset(const struct btf *btf, __u32 offset); > LIBBPF_API const char *btf__str_by_offset(const struct btf *btf, __u32 offset); > LIBBPF_API int btf__get_from_id(__u32 id, struct btf **btf); > +LIBBPF_API struct btf *btf__load_from_kernel_by_id(__u32 id); let's move this definition to after btf__parse() to keep all "constructors" together (we can move btf__get_from_id() there for completeness as well, I suppose). > LIBBPF_API int btf__get_map_kv_tids(const struct btf *btf, const char *map_name, > __u32 expected_key_size, > __u32 expected_value_size, > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 242e97892043..eff005b1eba1 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -9576,8 +9576,8 @@ static int libbpf_find_prog_btf_id(const char *name, __u32 attach_prog_fd) > { > struct bpf_prog_info_linear *info_linear; > struct bpf_prog_info *info; > - struct btf *btf = NULL; > int err = -EINVAL; > + struct btf *btf; > > info_linear = bpf_program__get_prog_info_linear(attach_prog_fd, 0); > err = libbpf_get_error(info_linear); > @@ -9591,7 +9591,8 @@ static int libbpf_find_prog_btf_id(const char *name, __u32 attach_prog_fd) > pr_warn("The target program doesn't have BTF\n"); > goto out; > } > - if (btf__get_from_id(info->btf_id, &btf)) { > + btf = btf__load_from_kernel_by_id(info->btf_id); > + if (libbpf_get_error(btf)) { there seems to be a bug in existing code and you are keeping it. On error err will be 0. Let's fix it. Same for above if (!info->btf_id), please fix that as well while you are at it. > pr_warn("Failed to get BTF of the program\n"); > goto out; > } > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map > index f7d52d76ca3a..ca8cc7a7faad 100644 > --- a/tools/lib/bpf/libbpf.map > +++ b/tools/lib/bpf/libbpf.map > @@ -373,6 +373,7 @@ LIBBPF_0.5.0 { > bpf_map__initial_value; > bpf_map_lookup_and_delete_elem_flags; > bpf_object__gen_loader; > + btf__load_from_kernel_by_id; > btf__load_into_kernel; > btf_dump__dump_type_data; > libbpf_set_strict_mode; > -- > 2.30.2 >
