Re: [PATCH bpf-next v5 4/9] bpftool: Add struct definitions and helpers for BTFGen

On Wed, Feb 2, 2022 at 1:55 PM Andrii Nakryiko
[off-list ref] wrote:

On Fri, Jan 28, 2022 at 2:33 PM Mauricio Vásquez [off-list ref] wrote:

quoted

Add some structs and helpers that will be used by BTFGen in the next
commits.

Signed-off-by: Mauricio Vásquez <redacted>
Signed-off-by: Rafael David Tinoco <redacted>
Signed-off-by: Lorenzo Fontana <redacted>
Signed-off-by: Leonardo Di Donato <redacted>
---

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.

quoted

 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.

quoted

+               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