lib-storage: Fix memory leak in mail_search_args_simplify()

The leaks happened when search args were already initialized (which they
usually were at this point) and some of the args were dropped entirely.

Improved the unit test to initialize search args before calling the
simplify, so valgrind would notice if there are any leaks.

diff --git a/src/lib-storage/mail-search-args-simplify.c b/src/lib-storage/mail-search-args-simplify.c
index 7c7a24e08cf90125a2c9cf48331dfb028d32f043..154383d7497bac6a7948ab498c8be5f36265f73e 100644 (file)
--- a/src/lib-storage/mail-search-args-simplify.c
+++ b/src/lib-storage/mail-search-args-simplify.c
@@ -27,6 +27,7 @@ struct mail_search_simplify_ctx {
                   struct mail_search_simplify_prev_arg *) prev_args;
        bool parent_and;
        bool removals;
+       bool initialized;
 };
 
 static int
@@ -96,6 +97,9 @@ mail_search_args_merge_mask(struct mail_search_simplify_ctx *ctx,
                *prev_argp = args;
                return FALSE;
        }
+       if (ctx->initialized)
+               mail_search_arg_deinit(*prev_argp);
+
        if ((*prev_argp)->match_not != args->match_not) {
                /* a && !a = 0 */
                (*prev_argp)->type = SEARCH_ALL;
@@ -326,7 +330,8 @@ mail_search_args_have_equal(const struct mail_search_arg *args,
 }
 
 static bool
-mail_search_args_remove_equal(struct mail_search_arg **argsp,
+mail_search_args_remove_equal(struct mail_search_args *all_args,
+                             struct mail_search_arg **argsp,
                              const struct mail_search_arg *wanted_arg,
                              bool check_subs)
 {
@@ -335,12 +340,14 @@ mail_search_args_remove_equal(struct mail_search_arg **argsp,
 
        for (argp = argsp; (*argp) != NULL; ) {
                if (mail_search_arg_one_equals(*argp, wanted_arg)) {
+                       if (all_args->init_refcount > 0)
+                               mail_search_arg_deinit(*argp);
                        *argp = (*argp)->next;
                        found = TRUE;
                } else if (check_subs) {
                        i_assert((*argp)->type == SEARCH_SUB ||
                                 (*argp)->type == SEARCH_OR);
-                       if (!mail_search_args_remove_equal(&(*argp)->value.subargs, wanted_arg, FALSE)) {
+                       if (!mail_search_args_remove_equal(all_args, &(*argp)->value.subargs, wanted_arg, FALSE)) {
                                /* we already verified that this should have
                                   existed. */
                                i_unreached();
@@ -384,7 +391,8 @@ mail_search_args_count(const struct mail_search_arg *args)
 }
 
 static bool
-mail_search_args_simplify_drop_redundant_args(struct mail_search_arg **argsp,
+mail_search_args_simplify_drop_redundant_args(struct mail_search_args *all_args,
+                                             struct mail_search_arg **argsp,
                                              bool and_arg)
 {
        struct mail_search_arg *arg, **argp, one_arg, *lowest_arg = NULL;
@@ -419,6 +427,8 @@ mail_search_args_simplify_drop_redundant_args(struct mail_search_arg **argsp,
                if (*argp != lowest_arg && (*argp)->type == child_subargs_type &&
                    (*argp)->value.subargs != lowest_arg &&
                    mail_search_args_have_all_equal(*argp, lowest_arg)) {
+                       if (all_args->init_refcount > 0)
+                               mail_search_arg_deinit(*argp);
                        *argp = (*argp)->next;
                        ret = TRUE;
                } else {
@@ -429,7 +439,8 @@ mail_search_args_simplify_drop_redundant_args(struct mail_search_arg **argsp,
 }
 
 static bool
-mail_search_args_simplify_extract_common(struct mail_search_arg **argsp,
+mail_search_args_simplify_extract_common(struct mail_search_args *all_args,
+                                        struct mail_search_arg **argsp,
                                         pool_t pool, bool and_arg)
 {
        /* Simple SUB example:
@@ -483,7 +494,7 @@ mail_search_args_simplify_extract_common(struct mail_search_arg **argsp,
                        continue;
 
                /* extract the arg and put it to common_args */
-               mail_search_args_remove_equal(argsp, sub_arg, TRUE);
+               mail_search_args_remove_equal(all_args, argsp, sub_arg, TRUE);
                sub_arg->next = common_args;
                common_args = sub_arg;
        }
@@ -511,7 +522,7 @@ mail_search_args_simplify_extract_common(struct mail_search_arg **argsp,
 }
 
 static bool
-mail_search_args_simplify_sub(struct mailbox *box, pool_t pool,
+mail_search_args_simplify_sub(struct mail_search_args *all_args, pool_t pool,
                              struct mail_search_arg **argsp, bool parent_and)
 {
        struct mail_search_simplify_ctx ctx;
@@ -519,6 +530,7 @@ mail_search_args_simplify_sub(struct mailbox *box, pool_t pool,
        bool merged;
 
        i_zero(&ctx);
+       ctx.initialized = all_args->init_refcount > 0;
        ctx.parent_and = parent_and;
        ctx.pool = pool_alloconly_create("mail search args simplify", 1024);
        hash_table_create(&ctx.prev_args, ctx.pool, 0,
@@ -565,12 +577,12 @@ mail_search_args_simplify_sub(struct mailbox *box, pool_t pool,
                        if (args->type != SEARCH_INTHREAD) {
                                bool and_arg = args->type == SEARCH_SUB;
 
-                               if (mail_search_args_simplify_drop_redundant_args(&args->value.subargs, and_arg))
+                               if (mail_search_args_simplify_drop_redundant_args(all_args, &args->value.subargs, and_arg))
                                        ctx.removals = TRUE;
-                               if (mail_search_args_simplify_extract_common(&args->value.subargs, pool, and_arg))
+                               if (mail_search_args_simplify_extract_common(all_args, &args->value.subargs, pool, and_arg))
                                        ctx.removals = TRUE;
                        }
-                       if (mail_search_args_simplify_sub(box, pool, &args->value.subargs,
+                       if (mail_search_args_simplify_sub(all_args, pool, &args->value.subargs,
                                                          args->type != SEARCH_OR))
                                ctx.removals = TRUE;
                }
@@ -759,20 +771,20 @@ void mail_search_args_simplify(struct mail_search_args *args)
 
        args->simplified = TRUE;
 
-       removals = mail_search_args_simplify_sub(args->box, args->pool, &args->args, TRUE);
+       removals = mail_search_args_simplify_sub(args, args->pool, &args->args, TRUE);
        if (mail_search_args_unnest_inthreads(args, &args->args,
                                              FALSE, TRUE)) {
                /* we may have added some extra SUBs that could be dropped */
-               if (mail_search_args_simplify_sub(args->box, args->pool, &args->args, TRUE))
+               if (mail_search_args_simplify_sub(args, args->pool, &args->args, TRUE))
                        removals = TRUE;
        }
        do {
-               if (mail_search_args_simplify_drop_redundant_args(&args->args, TRUE))
+               if (mail_search_args_simplify_drop_redundant_args(args, &args->args, TRUE))
                        removals = TRUE;
-               if (mail_search_args_simplify_extract_common(&args->args, args->pool, TRUE))
+               if (mail_search_args_simplify_extract_common(args, &args->args, args->pool, TRUE))
                        removals = TRUE;
                if (removals)
-                       removals = mail_search_args_simplify_sub(args->box, args->pool, &args->args, TRUE);
+                       removals = mail_search_args_simplify_sub(args, args->pool, &args->args, TRUE);
                /* do the flag merging into a single arg only at the end.
                   up until then they're treated as any other search args,
                   which simplifies their handling. after the flags merging is

diff --git a/src/lib-storage/test-mail-search-args-simplify.c b/src/lib-storage/test-mail-search-args-simplify.c
index b83697deac7a25742ab8796ce88ec0f417769dac..4862be83164879596b29bde1e2eec008f372ed10 100644 (file)
--- a/src/lib-storage/test-mail-search-args-simplify.c
+++ b/src/lib-storage/test-mail-search-args-simplify.c
@@ -3,6 +3,7 @@
 #include "lib.h"
 #include "str.h"
 #include "test-common.h"
+#include "mail-storage-private.h"
 #include "mail-search-build.h"
 #include "mail-search-parser.h"
 #include "mail-search.h"
@@ -237,13 +238,18 @@ test_build_search_args(const char *args)
 static void test_mail_search_args_simplify(void)
 {
        struct mail_search_args *args;
+       struct mail_storage_settings set = { .mail_max_keyword_length = 100 };
+       struct mail_storage storage = { .set = &set };
+       struct mailbox box = { .opened = TRUE, .storage = &storage };
        string_t *str = t_str_new(256);
        const char *error;
        unsigned int i;
 
        test_begin("mail search args simplify");
+       box.index = mail_index_alloc(NULL, "dovecot.index.");
        for (i = 0; i < N_ELEMENTS(tests); i++) {
                args = test_build_search_args(tests[i].input);
+               mail_search_args_init(args, &box, FALSE, NULL);
                mail_search_args_simplify(args);
 
                str_truncate(str, 0);
@@ -251,6 +257,7 @@ static void test_mail_search_args_simplify(void)
                test_assert_idx(strcmp(str_c(str), tests[i].output) == 0, i);
                mail_search_args_unref(&args);
        }
+       mail_index_free(&box.index);
        test_end();
 }
 
@@ -270,8 +277,10 @@ static void test_mail_search_args_simplify_empty_lists(void)
 int main(void)
 {
        static void (*const test_functions[])(void) = {
+               mail_storage_init,
                test_mail_search_args_simplify,
                test_mail_search_args_simplify_empty_lists,
+               mail_storage_deinit,
                NULL
        };