Re: [PATCH 14/19] libbpf: Add btf__find_by_pattern_kind function

[Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx>]

On Sat, Jun 5, 2021 at 4:14 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
>
> Adding btf__find_by_pattern_kind function that returns
> array of BTF ids for given function name pattern.
>
> Using libc's regex.h support for that.
>
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
>  tools/lib/bpf/btf.c | 68 +++++++++++++++++++++++++++++++++++++++++++++
>  tools/lib/bpf/btf.h |  3 ++
>  2 files changed, 71 insertions(+)
>
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index b46760b93bb4..421dd6c1e44a 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
>  /* Copyright (c) 2018 Facebook */
>
> +#define _GNU_SOURCE
>  #include <byteswap.h>
>  #include <endian.h>
>  #include <stdio.h>
> @@ -16,6 +17,7 @@
>  #include <linux/err.h>
>  #include <linux/btf.h>
>  #include <gelf.h>
> +#include <regex.h>
>  #include "btf.h"
>  #include "bpf.h"
>  #include "libbpf.h"
> @@ -711,6 +713,72 @@ __s32 btf__find_by_name_kind(const struct btf *btf, const char *type_name,
>         return libbpf_err(-ENOENT);
>  }
>
> +static bool is_wildcard(char c)
> +{
> +       static const char *wildchars = "*?[|";
> +
> +       return strchr(wildchars, c);
> +}
> +
> +int btf__find_by_pattern_kind(const struct btf *btf,
> +                             const char *type_pattern, __u32 kind,
> +                             __s32 **__ids)
> +{
> +       __u32 i, nr_types = btf__get_nr_types(btf);
> +       __s32 *ids = NULL;
> +       int cnt = 0, alloc = 0, ret;
> +       regex_t regex;
> +       char *pattern;
> +
> +       if (kind == BTF_KIND_UNKN || !strcmp(type_pattern, "void"))
> +               return 0;
> +
> +       /* When the pattern does not start with wildcard, treat it as
> +        * if we'd want to match it from the beginning of the string.
> +        */

This assumption is absolutely atrocious. If we say it's regexp, then
it has to always be regexp, not something based on some random
heuristic based on the first character.

Taking a step back, though. Do we really need to provide this API? Why
applications can't implement it on their own, given regexp
functionality is provided by libc. Which I didn't know, actually, so
that's pretty nice, assuming that it's also available in more minimal
implementations like musl.

> +       asprintf(&pattern, "%s%s",
> +                is_wildcard(type_pattern[0]) ? "^" : "",
> +                type_pattern);
> +
> +       ret = regcomp(&regex, pattern, REG_EXTENDED);
> +       if (ret) {
> +               pr_warn("failed to compile regex\n");
> +               free(pattern);
> +               return -EINVAL;
> +       }
> +
> +       free(pattern);
> +
> +       for (i = 1; i <= nr_types; i++) {
> +               const struct btf_type *t = btf__type_by_id(btf, i);
> +               const char *name;
> +               __s32 *p;
> +
> +               if (btf_kind(t) != kind)
> +                       continue;
> +               name = btf__name_by_offset(btf, t->name_off);
> +               if (name && regexec(&regex, name, 0, NULL, 0))
> +                       continue;
> +               if (cnt == alloc) {
> +                       alloc = max(100, alloc * 3 / 2);
> +                       p = realloc(ids, alloc * sizeof(__u32));

this memory allocation and re-allocation on behalf of users is another
argument against this API

> +                       if (!p) {
> +                               free(ids);
> +                               regfree(&regex);
> +                               return -ENOMEM;
> +                       }
> +                       ids = p;
> +               }
> +
> +               ids[cnt] = i;
> +               cnt++;
> +       }
> +
> +       regfree(&regex);
> +       *__ids = ids;
> +       return cnt ?: -ENOENT;
> +}
> +
>  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 b54f1c3ebd57..036857aded94 100644
> --- a/tools/lib/bpf/btf.h
> +++ b/tools/lib/bpf/btf.h
> @@ -371,6 +371,9 @@ btf_var_secinfos(const struct btf_type *t)
>         return (struct btf_var_secinfo *)(t + 1);
>  }
>
> +int btf__find_by_pattern_kind(const struct btf *btf,
> +                             const char *type_pattern, __u32 kind,
> +                             __s32 **__ids);
>  #ifdef __cplusplus
>  } /* extern "C" */
>  #endif
> --
> 2.31.1
>