On Thu, Sep 10, 2020 at 12:54 PM Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > On Wed, Sep 9, 2020 at 11:25 AM Stanislav Fomichev <sdf@xxxxxxxxxx> wrote: > > diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c > > index f7923414a052..ca264dc22434 100644 > > --- a/tools/bpf/bpftool/prog.c > > +++ b/tools/bpf/bpftool/prog.c > > @@ -29,6 +29,9 @@ > > #include "main.h" > > #include "xlated_dumper.h" > > > > +#define BPF_METADATA_PREFIX "bpf_metadata_" > > +#define BPF_METADATA_PREFIX_LEN strlen(BPF_METADATA_PREFIX) > > this is a runtime check, why not (sizeof(BPF_METADATA_PREFIX) - 1) instead? Make sense, will fix. > > +static int bpf_prog_find_metadata(int prog_fd, int *map_id) > > ... > > +free_map_ids: > > + saved_errno = errno; > > + free(map_ids); > > + errno = saved_errno; > > not clear why all this fussing with saving/restoring errno and then > just returning 0 or -1? Just return -ENOMEM or -ENOENT as a result of > this function? Yeah, I just moved this function from it's original (libbpf) location as is. I guess it makes sense to simplify the error handling now that it's not in exported from libbpf. > > + if (bpf_map_lookup_elem(map_fd, &key, value)) > > + goto out_free; > > + > > + err = btf__get_from_id(map_info.btf_id, &btf); > > + if (err || !btf) > > + goto out_free; > > if you make bpf_prog_find_metadata() to do this value lookup and pass > &info, it would probably make bpf_prog_find_metadata a bit more > usable? You'll just need to ensure that callers free allocated memory. > Then show_prog_metadata() would take care of processing BTF info. Sounds reasonable. I can maybe keep existing bpf_prog_find_metadata (rename to bpf_prog_find_metadata_map_id?) and add another wrapper that does that map lookup.
