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

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Wed, Jul 22, 2020 at 2:13 PM Jiri Olsa <jolsa@kernel.org> wrote:
>
> 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?

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

> +       if (!id) {
> +               pr_err("FAILED to parse cnt name: %s\n", name);
> +               return NULL;
> +       }
> +
> +       return btf_id__add(&obj->sets, id, true);
> +}
> +
>  static struct btf_id *add_symbol(struct rb_root *root, char *name, size_t size)
>  {
>         char *id;
> @@ -376,7 +389,7 @@ static int symbols_collect(struct object *obj)
>                         id = add_symbol(&obj->funcs, prefix, sizeof(BTF_FUNC) - 1);
>                 /* set */
>                 } else if (!strncmp(prefix, BTF_SET, sizeof(BTF_SET) - 1)) {
> -                       id = add_symbol(&obj->sets, prefix, sizeof(BTF_SET) - 1);
> +                       id = add_set(obj, prefix);
>                         /*
>                          * SET objects store list's count, which is encoded
>                          * in symbol's size, together with 'cnt' field hence
> --
> 2.25.4
>