Re: [PATCH bpf-next v4 20/27] libbpf: Add btf__find_by_glob_kind function

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

On Thu, Aug 26, 2021 at 12:41 PM Jiri Olsa <jolsa@redhat.com> wrote:
>
> Adding btf__find_by_glob_kind function that returns array of
> BTF ids that match given kind and allow/deny patterns.
>
> int btf__find_by_glob_kind(const struct btf *btf, __u32 kind,
>                            const char *allow_pattern,
>                            const char *deny_pattern,
>                            __u32 **__ids);
>
> The __ids array is allocated and needs to be manually freed.
>
> At the moment the supported pattern is '*' at the beginning or
> the end of the pattern.
>
> Kindly borrowed from retsnoop.
>
> Suggested-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/lib/bpf/btf.c | 80 +++++++++++++++++++++++++++++++++++++++++++++
>  tools/lib/bpf/btf.h |  3 ++
>  2 files changed, 83 insertions(+)
>
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index 77dc24d58302..5baaca6c3134 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -711,6 +711,86 @@ __s32 btf__find_by_name_kind(const struct btf *btf, const char *type_name,
>         return libbpf_err(-ENOENT);
>  }
>
> +/* 'borrowed' from retsnoop */
> +static bool glob_matches(const char *glob, const char *s)
> +{
> +       int n = strlen(glob);
> +
> +       if (n == 1 && glob[0] == '*')
> +               return true;
> +
> +       if (glob[0] == '*' && glob[n - 1] == '*') {
> +               const char *subs;
> +               /* substring match */
> +
> +               /* this is hacky, but we don't want to allocate for no good reason */
> +               ((char *)glob)[n - 1] = '\0';
> +               subs = strstr(s, glob + 1);
> +               ((char *)glob)[n - 1] = '*';
> +
> +               return subs != NULL;
> +       } else if (glob[0] == '*') {
> +               size_t nn = strlen(s);
> +               /* suffix match */
> +
> +               /* too short for a given suffix */
> +               if (nn < n - 1)
> +                       return false;
> +
> +               return strcmp(s + nn - (n - 1), glob + 1) == 0;
> +       } else if (glob[n - 1] == '*') {
> +               /* prefix match */
> +               return strncmp(s, glob, n - 1) == 0;
> +       } else {
> +               /* exact match */
> +               return strcmp(glob, s) == 0;
> +       }
> +}
> +
> +int btf__find_by_glob_kind(const struct btf *btf, __u32 kind,
> +                          const char *allow_pattern, const char *deny_pattern,
> +                          __u32 **__ids)
> +{
> +       __u32 i, nr_types = btf__get_nr_types(btf);
> +       int cnt = 0, alloc = 0;
> +       __u32 *ids = NULL;
> +
> +       for (i = 1; i <= nr_types; i++) {
> +               const struct btf_type *t = btf__type_by_id(btf, i);
> +               bool match = false;
> +               const char *name;
> +               __u32 *p;
> +
> +               if (btf_kind(t) != kind)
> +                       continue;
> +               name = btf__name_by_offset(btf, t->name_off);
> +               if (!name)
> +                       continue;
> +
> +               if (allow_pattern && glob_matches(allow_pattern, name))
> +                       match = true;
> +               if (deny_pattern && !glob_matches(deny_pattern, name))
> +                       match = true;

this is wrong, if it matches both deny and allow patterns, you'll
still pass it through. Drop the match flag, just check deny first and
`continue` if matches.

> +               if (!match)
> +                       continue;
> +
> +               if (cnt == alloc) {
> +                       alloc = max(100, alloc * 3 / 2);

nit: maybe start with something like 16?

> +                       p = realloc(ids, alloc * sizeof(__u32));

we have libbpf_reallocarray, please use it

> +                       if (!p) {
> +                               free(ids);
> +                               return -ENOMEM;
> +                       }
> +                       ids = p;
> +               }
> +               ids[cnt] = i;
> +               cnt++;
> +       }
> +
> +       *__ids = ids;
> +       return cnt ?: -ENOENT;

cnt == 0 means -ENOENT, basically, no? It's up to the application to
decide if that's an error, let's just return the number of matches,
i.e., zero.

> +}
> +
>  static bool btf_is_modifiable(const struct btf *btf)
>  {
>         return (void *)btf->hdr != btf->raw_data;
> diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
> index 4a711f990904..b288211770c3 100644
> --- a/tools/lib/bpf/btf.h
> +++ b/tools/lib/bpf/btf.h
> @@ -396,6 +396,9 @@ btf_var_secinfos(const struct btf_type *t)
>         return (struct btf_var_secinfo *)(t + 1);
>  }
>
> +int btf__find_by_glob_kind(const struct btf *btf, __u32 kind,
> +                          const char *allow_pattern, const char *deny_pattern,
> +                          __u32 **__ids);
>  #ifdef __cplusplus
>  } /* extern "C" */
>  #endif
> --
> 2.31.1
>