Re: [PATCH bpf-next 1/2] selftests/bpf: Support checks against a regular expression.

[Eduard Zingerman <eddyz87@gmail.com>]

On Mon, 2024-06-03 at 16:53 +0100, Cupertino Miranda wrote:

I think this macro is a long overdue, thank you for working on this.
A few notes below.

[...]

> +static int push_regex(const char *regex_str, struct test_subspec *subspec)
> +{
> +	void *tmp;
> +	int regcomp_res;
> +
> +	tmp = realloc(subspec->expect,
> +		      (1 + subspec->expect_msg_cnt) * sizeof(struct expect_msg));
> +	if (!tmp) {
> +		ASSERT_FAIL("failed to realloc memory for messages\n");
> +		return -ENOMEM;
> +	}
> +	subspec->expect = tmp;
> +
> +	subspec->expect[subspec->expect_msg_cnt].regex = (regex_t *) malloc(sizeof(regex_t));
> +	regcomp_res = regcomp (subspec->expect[subspec->expect_msg_cnt].regex,
> +			       regex_str, REG_EXTENDED|REG_NEWLINE);
> +	if (regcomp_res != 0) {
> +		fprintf(stderr, "Regexp: '%s'\n", regex_str);
> +		ASSERT_FAIL("failed to compile regex\n");
> +		return -EINVAL;
> +	}

Maybe also use a regerror() function that could be used to print
what's wrong with the regex.
Also, there is a ctx_rewrite.c:compile_regex, it might be interesting
to extract in from ctx_rewrite.c to testing_helpers.c and use it here.

> +
> +	subspec->expect[subspec->expect_msg_cnt].msg = regex_str;
> +	subspec->expect_msg_cnt += 1;
>  	return 0;
>  }

[...]

> @@ -403,26 +453,44 @@ static void validate_case(struct test_loader *tester,
>  			  int load_err)
>  {
>  	int i, j;
> +	const char *match;
>  
>  	for (i = 0; i < subspec->expect_msg_cnt; i++) {
> -		char *match;
>  		const char *expect_msg;
> +		regex_t *regex;
> +		regmatch_t reg_match[1];
> +
> +		expect_msg = subspec->expect[i].msg;
> +		regex = subspec->expect[i].regex;
> +
> +		if (regex == NULL) {
> +			match = strstr(tester->log_buf + tester->next_match_pos, expect_msg);
> +			if (!ASSERT_OK_PTR (match, "expect_msg")) {
> +				/* if we are in verbose mode, we've already emitted log */
> +				if (env.verbosity == VERBOSE_NONE)
> +					emit_verifier_log(tester->log_buf, true /*force*/);
> +				for (j = 0; j < i; j++)
> +					fprintf(stderr,
> +						"MATCHED  MSG: '%s'\n", subspec->expect[j].msg);
> +				fprintf(stderr, "EXPECTED MSG: '%s'\n", expect_msg);
> +				return;
> +			}
> +			tester->next_match_pos = match - tester->log_buf + strlen(expect_msg);
> +		} else {
> +			int match_size = regexec (regex, tester->log_buf + tester->next_match_pos, 1, reg_match, 0);
                                                ^
Nit:                                            |
        I think scripts/checkpatch.pl complains about such spaces

[...]

There is no regfree() call in the patch-set,
could you please extend free_test_spec()?