On Tue, 2024-06-11 at 18:40 +0100, Cupertino Miranda wrote:
> Add support for __regex and __regex_unpriv macros to check the test
> execution output against a regular expression. This is similar to __msg
> and __msg_unpriv, however those expect full text matching.
>
> Signed-off-by: Cupertino Miranda <cupertino.miranda@xxxxxxxxxx>
> Cc: jose.marchesi@xxxxxxxxxx
> Cc: david.faust@xxxxxxxxxx
> Cc: Yonghong Song <yonghong.song@xxxxxxxxx>
> Cc: Eduard Zingerman <eddyz87@xxxxxxxxx>
> Cc: Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx>
> ---
Overall looks good, could you please fix a few things noted below and respin?
Please add my ack on the respin.
Acked-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
[...]
> @@ -89,6 +98,16 @@ void test_loader_fini(struct test_loader *tester)
>
> static void free_test_spec(struct test_spec *spec)
> {
> + int i;
> +
> + /* Deallocate expect_msgs arrays. */
> + for (i = 0; i < spec->priv.expect_msg_cnt; i++)
> + if (spec->priv.expect_msgs && spec->priv.expect_msgs[i].regex_str)
I don't think situation when spec->priv.expect_msg_cnt > 0 and
spec->priv.expect_msgs == NULL is possible, conditions above and below
could be simplified to just "if (spec->[un]priv.expect_msgs[i].regex_str)"
> + regfree(&spec->priv.expect_msgs[i].regex);
> + for (i = 0; i < spec->unpriv.expect_msg_cnt; i++)
> + if (spec->unpriv.expect_msgs && spec->unpriv.expect_msgs[i].regex_str)
> + regfree(&spec->unpriv.expect_msgs[i].regex);
> +
> free(spec->priv.name);
> free(spec->unpriv.name);
> free(spec->priv.expect_msgs);
> @@ -100,17 +119,38 @@ static void free_test_spec(struct test_spec *spec)
> spec->unpriv.expect_msgs = NULL;
> }
>
> -static int push_msg(const char *msg, struct test_subspec *subspec)
> +static int push_msg(const char *substr, const char *regex_str, struct test_subspec *subspec)
> {
> void *tmp;
> + int regcomp_res;
> + char error_msg[100];
> + struct expect_msg *msg;
>
> - tmp = realloc(subspec->expect_msgs, (1 + subspec->expect_msg_cnt) * sizeof(void *));
> + tmp = realloc(subspec->expect_msgs,
> + (1 + subspec->expect_msg_cnt) * sizeof(struct expect_msg));
> if (!tmp) {
> ASSERT_FAIL("failed to realloc memory for messages\n");
> return -ENOMEM;
> }
> subspec->expect_msgs = tmp;
> - subspec->expect_msgs[subspec->expect_msg_cnt++] = msg;
> + msg = &subspec->expect_msgs[subspec->expect_msg_cnt];
> + subspec->expect_msg_cnt += 1;
> +
> + if (substr) {
> + msg->substr = substr;
> + msg->regex_str = NULL;
> + } else {
> + msg->regex_str = regex_str;
> + msg->substr = NULL;
> + regcomp_res = regcomp(&msg->regex, regex_str, REG_EXTENDED|REG_NEWLINE);
> + if (regcomp_res != 0) {
> + regerror(regcomp_res, &msg->regex, error_msg, 100);
^^^^
Nit: sizeof(error_msg)
> + fprintf(stderr, "Regexp compilation error in '%s': '%s'\n",
> + regex_str, error_msg);
> + ASSERT_FAIL("failed to compile regex\n");
Nit: these two calls could be combined as a single PRINT_FAIL().
> + return -EINVAL;
> + }
> + }
>
> return 0;
> }
[...]
> @@ -337,16 +389,11 @@ static int parse_test_spec(struct test_loader *tester,
> }
>
> if (!spec->unpriv.expect_msgs) {
> - size_t sz = spec->priv.expect_msg_cnt * sizeof(void *);
> + for (i = 0; i < spec->priv.expect_msg_cnt; i++) {
> + struct expect_msg *msg = &spec->priv.expect_msgs[i];
>
> - spec->unpriv.expect_msgs = malloc(sz);
> - if (!spec->unpriv.expect_msgs) {
> - PRINT_FAIL("failed to allocate memory for unpriv.expect_msgs\n");
> - err = -ENOMEM;
> - goto cleanup;
> + push_msg(msg->substr, msg->regex_str, &spec->unpriv);
Need to check push_msg() return value.
> }
> - memcpy(spec->unpriv.expect_msgs, spec->priv.expect_msgs, sz);
> - spec->unpriv.expect_msg_cnt = spec->priv.expect_msg_cnt;
> }
> }
>
[...]
