From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Yucong Sun <fallentree@fb.com>
Cc: Andrii Nakryiko <andrii@kernel.org>,
sunyucong@gmail.com, bpf <bpf@vger.kernel.org>
Subject: Re: [PATCH v5 bpf-next 4/4] selftests/bpf: Support glob matching for test selector.
Date: Tue, 17 Aug 2021 11:36:23 -0700 [thread overview]
Message-ID: <CAEf4BzYfbpBSn8aqUAm=pHNO_vp4n=A6p-CPgAKBhFmuGTueDA@mail.gmail.com> (raw)
In-Reply-To: <20210817044732.3263066-5-fallentree@fb.com>
On Mon, Aug 16, 2021 at 9:47 PM Yucong Sun <fallentree@fb.com> wrote:
>
> This patch adds '-a' and '-d' arguments, support exact string match, as well as
> using '*' wildcard in test/subtests selection. The old '-t' '-b' arguments
> still supports partial string match, but they can't be used together yet.
>
> This patach also adds support for mulitple '-a' '-d' '-t' '-b' arguments.
>
> Caveat: Same as the current substring matching mechanism, test and subtest
> selector applies independently, 'a*/b*' will execute all tests matching "a*",
> and with subtest name matching "b*", but tests matching "a*" that has no
> subtests will also be executed.
>
> Signed-off-by: Yucong Sun <fallentree@fb.com>
> ---
> tools/testing/selftests/bpf/test_progs.c | 72 +++++++++++++++++++-----
> 1 file changed, 58 insertions(+), 14 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index 90539b15b744..c34eb818f115 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -13,6 +13,28 @@
> #include <execinfo.h> /* backtrace */
> #include <linux/membarrier.h>
>
> +/* Adapted from perf/util/string.c */
> +static bool glob_match(const char *str, const char *pat)
> +{
> + while (*str && *pat && *pat != '*') {
> + if (*str != *pat)
> + return false;
> + str++;
> + pat++;
> + }
> + /* Check wild card */
> + if (*pat == '*') {
> + while (*pat == '*')
> + pat++;
> + if (!*pat) /* Tail wild card matches all */
> + return true;
> + while (*str)
> + if (glob_match(str++, pat))
> + return true;
> + }
> + return !*str && !*pat;
> +}
> +
> #define EXIT_NO_TEST 2
> #define EXIT_ERR_SETUP_INFRA 3
>
> @@ -55,12 +77,12 @@ static bool should_run(struct test_selector *sel, int num, const char *name)
> int i;
>
> for (i = 0; i < sel->blacklist.cnt; i++) {
> - if (strstr(name, sel->blacklist.strs[i]))
> + if (glob_match(name, sel->blacklist.strs[i]))
> return false;
> }
>
> for (i = 0; i < sel->whitelist.cnt; i++) {
> - if (strstr(name, sel->whitelist.strs[i]))
> + if (glob_match(name, sel->whitelist.strs[i]))
> return true;
> }
>
> @@ -450,6 +472,8 @@ enum ARG_KEYS {
> ARG_VERBOSE = 'v',
> ARG_GET_TEST_CNT = 'c',
> ARG_LIST_TEST_NAMES = 'l',
> + ARG_TEST_NAME_GLOB_ALLOWLIST = 'a',
> + ARG_TEST_NAME_GLOB_DENYLIST = 'd',
> };
>
> static const struct argp_option opts[] = {
> @@ -467,6 +491,10 @@ static const struct argp_option opts[] = {
> "Get number of selected top-level tests " },
> { "list", ARG_LIST_TEST_NAMES, NULL, 0,
> "List test names that would run (without running them) " },
> + { "allow", ARG_TEST_NAME_GLOB_ALLOWLIST, "NAMES", 0,
> + "Run tests with name matching the pattern (support *)." },
> + { "deny", ARG_TEST_NAME_GLOB_DENYLIST, "NAMES", 0,
> + "Don't run tests with name matching the pattern (support *)." },
> {},
> };
>
> @@ -491,7 +519,7 @@ static void free_str_set(const struct str_set *set)
> free(set->strs);
> }
>
> -static int parse_str_list(const char *s, struct str_set *set)
> +static int parse_str_list(const char *s, struct str_set *set, bool is_glob_pattern)
> {
> char *input, *state = NULL, *next, **tmp, **strs = NULL;
> int cnt = 0;
> @@ -500,28 +528,38 @@ static int parse_str_list(const char *s, struct str_set *set)
> if (!input)
> return -ENOMEM;
>
> - set->cnt = 0;
> - set->strs = NULL;
> -
> while ((next = strtok_r(state ? NULL : input, ",", &state))) {
> tmp = realloc(strs, sizeof(*strs) * (cnt + 1));
> if (!tmp)
> goto err;
> strs = tmp;
> + if (is_glob_pattern) {
> + strs[cnt] = strdup(next);
> + } else {
> + strs[cnt] = malloc(strlen(next) + 2 + 1);
> + if (!strs[cnt])
> + goto err;
> + sprintf(strs[cnt], "*%s*", next);
> + }
>
> - strs[cnt] = strdup(next);
> if (!strs[cnt])
> goto err;
>
> cnt++;
> }
>
> - set->cnt = cnt;
> - set->strs = (const char **)strs;
> + tmp = realloc(set->strs, sizeof(*strs) * (cnt + set->cnt));
> + if (!tmp)
> + goto err;
> + memcpy(tmp + set->cnt, strs, sizeof(*strs) * (cnt));
> + set->strs = (const char **)tmp;
> + set->cnt += cnt;
> free(input);
> + free(strs);
> return 0;
> err:
> - free(strs);
> + if (strs)
> + free(strs);
free(NULL) is noop, so no need to check if(strs)
You also missed the need to free all those strdup()'ed strings, so I added
for (i = 0; i < cnt; i++)
free(strs[i]);
We didn't need it originally, because eventually we call
free_str_set(), but in this case those strings will be leaked, because
we don't assign strs to set->strs.
BTW, a bit cleaner approach would be to just work on set->strs
directly, and realloc()ing it as necessary, and only at the end
updating set->cnt. You wouldn't need to memcpy and realloc one extra
time at the end, and no need for that for + free loop. But it's fine
the way it is as well.
Applied to bpf-next, thanks a lot for the improvements.
> free(input);
> return -ENOMEM;
> }
> @@ -553,29 +591,35 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
> }
> break;
> }
> + case ARG_TEST_NAME_GLOB_ALLOWLIST:
> case ARG_TEST_NAME: {
> char *subtest_str = strchr(arg, '/');
>
> if (subtest_str) {
> *subtest_str = '\0';
> if (parse_str_list(subtest_str + 1,
> - &env->subtest_selector.whitelist))
> + &env->subtest_selector.whitelist,
> + key == ARG_TEST_NAME_GLOB_ALLOWLIST))
> return -ENOMEM;
> }
> - if (parse_str_list(arg, &env->test_selector.whitelist))
> + if (parse_str_list(arg, &env->test_selector.whitelist,
> + key == ARG_TEST_NAME_GLOB_ALLOWLIST))
> return -ENOMEM;
> break;
> }
> + case ARG_TEST_NAME_GLOB_DENYLIST:
> case ARG_TEST_NAME_BLACKLIST: {
> char *subtest_str = strchr(arg, '/');
>
> if (subtest_str) {
> *subtest_str = '\0';
> if (parse_str_list(subtest_str + 1,
> - &env->subtest_selector.blacklist))
> + &env->subtest_selector.blacklist,
> + key == ARG_TEST_NAME_GLOB_DENYLIST))
> return -ENOMEM;
> }
> - if (parse_str_list(arg, &env->test_selector.blacklist))
> + if (parse_str_list(arg, &env->test_selector.blacklist,
> + key == ARG_TEST_NAME_GLOB_DENYLIST))
> return -ENOMEM;
> break;
> }
> --
> 2.30.2
>
prev parent reply other threads:[~2021-08-17 18:36 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-17 4:47 [PATCH v5 bpf-next 0/4] selftests/bpf: Improve the usability of test_progs Yucong Sun
2021-08-17 4:47 ` [PATCH v5 bpf-next 1/4] selftests/bpf: skip loading bpf_testmod when using -l to list tests Yucong Sun
2021-08-17 4:47 ` [PATCH v5 bpf-next 2/4] selftests/bpf: correctly display subtest skip status Yucong Sun
2021-08-17 4:47 ` [PATCH v5 bpf-next 3/4] selftests/bpf: also print test name in subtest status message Yucong Sun
2021-08-17 4:47 ` [PATCH v5 bpf-next 4/4] selftests/bpf: Support glob matching for test selector Yucong Sun
2021-08-17 18:36 ` Andrii Nakryiko [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAEf4BzYfbpBSn8aqUAm=pHNO_vp4n=A6p-CPgAKBhFmuGTueDA@mail.gmail.com' \
--to=andrii.nakryiko@gmail.com \
--cc=andrii@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=fallentree@fb.com \
--cc=sunyucong@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.