On Wed, Feb 2, 2022 at 1:55 PM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Fri, Jan 28, 2022 at 2:33 PM Mauricio Vásquez <mauricio@xxxxxxxxxx> wrote:
> >
> > Add some structs and helpers that will be used by BTFGen in the next
> > commits.
> >
> > Signed-off-by: Mauricio Vásquez <mauricio@xxxxxxxxxx>
> > Signed-off-by: Rafael David Tinoco <rafael.tinoco@xxxxxxxxxxx>
> > Signed-off-by: Lorenzo Fontana <lorenzo.fontana@xxxxxxxxxx>
> > Signed-off-by: Leonardo Di Donato <leonardo.didonato@xxxxxxxxxx>
> > ---
>
> Similar considerations with unused static functions. It's also harder
> to review when I don't see how these types are actually used, so
> probably better to put it in relevant patches that are using this?
>
The next iteration splits the patches in a way that types are
introduced in the same commit they're used.
> > tools/bpf/bpftool/gen.c | 75 +++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 75 insertions(+)
> >
> > diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> > index 64371f466fa6..68bb88e86b27 100644
> > --- a/tools/bpf/bpftool/gen.c
> > +++ b/tools/bpf/bpftool/gen.c
> > @@ -1118,6 +1118,81 @@ static int btf_save_raw(const struct btf *btf, const char *path)
> > return err;
> > }
> >
> > +struct btfgen_type {
> > + struct btf_type *type;
> > + unsigned int id;
> > +};
> > +
> > +struct btfgen_info {
> > + struct hashmap *types;
> > + struct btf *src_btf;
> > +};
> > +
> > +static size_t btfgen_hash_fn(const void *key, void *ctx)
> > +{
> > + return (size_t)key;
> > +}
> > +
> > +static bool btfgen_equal_fn(const void *k1, const void *k2, void *ctx)
> > +{
> > + return k1 == k2;
> > +}
> > +
> > +static void *uint_as_hash_key(int x)
> > +{
> > + return (void *)(uintptr_t)x;
> > +}
> > +
> > +static void btfgen_free_type(struct btfgen_type *type)
> > +{
> > + free(type);
> > +}
> > +
> > +static void btfgen_free_info(struct btfgen_info *info)
> > +{
> > + struct hashmap_entry *entry;
> > + size_t bkt;
> > +
> > + if (!info)
> > + return;
> > +
> > + if (!IS_ERR_OR_NULL(info->types)) {
> > + hashmap__for_each_entry(info->types, entry, bkt) {
> > + btfgen_free_type(entry->value);
> > + }
> > + hashmap__free(info->types);
> > + }
> > +
> > + btf__free(info->src_btf);
> > +
> > + free(info);
> > +}
> > +
> > +static struct btfgen_info *
> > +btfgen_new_info(const char *targ_btf_path)
> > +{
> > + struct btfgen_info *info;
> > +
> > + info = calloc(1, sizeof(*info));
> > + if (!info)
> > + return NULL;
> > +
> > + info->src_btf = btf__parse(targ_btf_path, NULL);
> > + if (libbpf_get_error(info->src_btf)) {
>
> bpftool is using libbpf 1.0 mode, so don't use libbpf_get_error()
> anymore, just check for NULL
>
hmm, I got confused because libbpf_get_error() is still used in many
places in bpftool. I suppose those need to be updated.
> also, if you are using errno for propagating error, you need to store
> it locally before btfgen_free_info() call, otherwise it can be
> clobbered
>
Fixed.
> > + btfgen_free_info(info);
> > + return NULL;
> > + }
> > +
> > + info->types = hashmap__new(btfgen_hash_fn, btfgen_equal_fn, NULL);
> > + if (IS_ERR(info->types)) {
> > + errno = -PTR_ERR(info->types);
> > + btfgen_free_info(info);
> > + return NULL;
> > + }
> > +
> > + return info;
> > +}
> > +
> > /* Create BTF file for a set of BPF objects */
> > static int btfgen(const char *src_btf, const char *dst_btf, const char *objspaths[])
> > {
> > --
> > 2.25.1
> >
