Re: [PATCH v8 bpf-next 02/13] tools resolve_btfids: Add support for set symbols

On Mon, Jul 27, 2020 at 05:53:01PM -0700, Andrii Nakryiko wrote:

On Wed, Jul 22, 2020 at 2:13 PM Jiri Olsa [off-list ref] wrote:

quoted

The set symbol does not have the unique number suffix,
so we need to give it a special parsing function.

This was omitted in the first batch, because there was
no set support yet, so it slipped in the testing.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/bpf/resolve_btfids/main.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
index 6956b6350cad..c28ab0401818 100644
--- a/tools/bpf/resolve_btfids/main.c
+++ b/tools/bpf/resolve_btfids/main.c

@@ -220,6 +220,19 @@ static char *get_id(const char *prefix_end)
        return id;
 }

+static struct btf_id *add_set(struct object *obj, char *name)
+{
+       char *id;
+
+       id = strdup(name + sizeof(BTF_SET) + sizeof("__") - 2);

why strdup? you are not really managing memory carefully anyway,
letting OS clean everything up, so why bother strduping here?

it copies the get_id logic, where we cut the unique ID part,
but we don't cut the string in here, so no reason for strdup
I'll remove it

Also if get invalid identifier, you can easily go past the string and
its ending zero byte. So check strlen first?

right.. it's also missing in get_id funciton, will add

thanks,
jirka

quoted

+       if (!id) {
+               pr_err("FAILED to parse cnt name: %s\n", name);
+               return NULL;
+       }
+
+       return btf_id__add(&obj->sets, id, true);
+}
+

SNIP