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

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(-)

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
>

On Mon, Jul 27, 2020 at 05:53:01PM -0700, Andrii Nakryiko wrote:
> 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?

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

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

SNIP