]> git.ipfire.org Git - thirdparty/dovecot/core.git/commitdiff
lib: Changed t_pop() API to make it a bit more like free()
authorTimo Sirainen <timo.sirainen@dovecot.fi>
Thu, 1 Sep 2016 05:24:19 +0000 (08:24 +0300)
committerGitLab <gitlab@git.dovecot.net>
Fri, 2 Sep 2016 07:50:29 +0000 (10:50 +0300)
src/lib-master/master-service.c
src/lib-test/test-common.c
src/lib/data-stack.c
src/lib/data-stack.h
src/lib/ioloop.c
src/lib/test-data-stack.c

index edbb2cb1cab4615e5a6368cfc7699cc0a3f01fb8..9294f059f3c2f05b9d98932dc9717f53da9bbf37 100644 (file)
@@ -535,7 +535,7 @@ void master_service_init_finish(struct master_service *service)
 
        /* close data stack frame opened by master_service_init() */
        if ((service->flags & MASTER_SERVICE_FLAG_NO_INIT_DATASTACK_FRAME) == 0) {
-               if (t_pop() != service->datastack_frame_id)
+               if (!t_pop(&service->datastack_frame_id))
                        i_panic("Leaked t_pop() call");
        }
 }
index c8568014e478af80560e70b1b9aa3032dba3164b..b026492d5215d8d93c8510e797d5a0da74fc4ea7 100644 (file)
@@ -454,9 +454,11 @@ int test_run_named_with_fatals(const char *match, struct named_test tests[],
 void ATTR_NORETURN
 test_exit(int status)
 {
+       data_stack_frame_t id = 0;
+
        i_free_and_null(expected_error_str);
        i_free_and_null(test_prefix);
-       (void)t_pop(); /* as we were within a T_BEGIN { tests[i].func(); } T_END */
+       (void)t_pop(&id); /* as we were within a T_BEGIN { tests[i].func(); } T_END */
        lib_deinit();
        _exit(status);
 }
index 8e1b53bffa76a76398a64d5a67ef70e3d4f30615..7e071502300ffc99d68ddc2bd28292a0fe6b635f 100644 (file)
@@ -70,6 +70,8 @@ struct stack_frame_block {
 data_stack_frame_t data_stack_frame = 0;
 
 static bool data_stack_initialized = FALSE;
+static data_stack_frame_t root_frame_id;
+
 static int frame_pos = BLOCK_FRAME_COUNT-1; /* in current_frame_block */
 static struct stack_frame_block *current_frame_block;
 static struct stack_frame_block *unused_frame_blocks;
@@ -260,7 +262,7 @@ static void t_pop_verify(void)
 }
 #endif
 
-data_stack_frame_t t_pop(void)
+bool t_pop(data_stack_frame_t *id)
 {
        struct stack_frame_block *frame_block;
 
@@ -306,15 +308,10 @@ data_stack_frame_t t_pop(void)
                frame_block->prev = unused_frame_blocks;
                unused_frame_blocks = frame_block;
        }
-
-       return --data_stack_frame;
-}
-
-void t_pop_check(data_stack_frame_t *id)
-{
-       if (unlikely(t_pop() != *id))
-               i_panic("Leaked t_pop() call");
+       if (unlikely(--data_stack_frame != *id))
+               return FALSE;
        *id = 0;
+       return TRUE;
 }
 
 static struct stack_block *mem_block_alloc(size_t min_size)
@@ -590,14 +587,13 @@ void data_stack_init(void)
        last_buffer_block = NULL;
        last_buffer_size = 0;
 
-       (void)t_push("data_stack_init");
+       root_frame_id = t_push("data_stack_init");
 }
 
 void data_stack_deinit(void)
 {
-       (void)t_pop();
-
-       if (frame_pos != BLOCK_FRAME_COUNT-1)
+       if (!t_pop(&root_frame_id) ||
+           frame_pos != BLOCK_FRAME_COUNT-1)
                i_panic("Missing t_pop() call");
 
 #ifndef USE_GC
index 14aeb81385bf6e2ce58366c9e0ec795b656d809f..6867c08a9a4cc55cd72670c5e8163a0bc531babf 100644 (file)
@@ -39,7 +39,7 @@ extern data_stack_frame_t data_stack_frame;
    is called. Returns the current stack frame number, which can be used
    to detect missing t_pop() calls:
 
-   x = t_push(__func__); .. if (t_pop() != x) abort();
+   x = t_push(__func__); .. if (!t_pop(x)) abort();
 
    In DEBUG mode, t_push_named() makes a temporary allocation for the name,
    but is safe to call in a loop as it performs the allocation within its own
@@ -47,10 +47,9 @@ extern data_stack_frame_t data_stack_frame;
 */
 data_stack_frame_t t_push(const char *marker) ATTR_HOT;
 data_stack_frame_t t_push_named(const char *format, ...) ATTR_HOT ATTR_FORMAT(1, 2);
-data_stack_frame_t t_pop(void) ATTR_HOT;
-/* Simplifies the if (t_pop() != x) check by comparing it internally and
-   panicking if it doesn't match. */
-void t_pop_check(data_stack_frame_t *id) ATTR_HOT;
+/* Returns TRUE on success, FALSE if t_pop() call was leaked. The caller
+   should panic. */
+bool t_pop(data_stack_frame_t *id) ATTR_HOT;
 
 /* Usage: T_BEGIN { code } T_END */
 #ifndef DEBUG
@@ -65,7 +64,11 @@ void t_pop_check(data_stack_frame_t *id) ATTR_HOT;
        STMT_START { data_stack_frame_t _data_stack_cur_id = t_push(T_CAT2(__FILE__,__LINE__));
 #endif
 #define T_END \
-       t_pop_check(&_data_stack_cur_id); } STMT_END
+       STMT_START { \
+               if (unlikely(!t_pop(&_data_stack_cur_id))) \
+                       i_panic("Leaked t_pop() call"); \
+       } STMT_END; \
+       } STMT_END
 
 /* WARNING: Be careful when using these functions, it's too easy to
    accidentally save the returned value somewhere permanently.
index 91da3830ae770ee2e2bad1bc4d8b586c7318cfaf..e006da0d8c6d4762ed1927bf90d3e576c7dd2b80 100644 (file)
@@ -530,7 +530,7 @@ static void io_loop_handle_timeouts_real(struct ioloop *ioloop)
                t_id = t_push_named("ioloop timeout handler %p",
                                    (void *)timeout->callback);
                timeout->callback(timeout->context);
-               if (t_pop() != t_id) {
+               if (!t_pop(&t_id)) {
                        i_panic("Leaked a t_pop() call in timeout handler %p",
                                (void *)timeout->callback);
                }
@@ -562,7 +562,7 @@ void io_loop_call_io(struct io *io)
        t_id = t_push_named("ioloop handler %p",
                            (void *)io->callback);
        io->callback(io->context);
-       if (t_pop() != t_id) {
+       if (!t_pop(&t_id)) {
                i_panic("Leaked a t_pop() call in I/O handler %p",
                        (void *)io->callback);
        }
index 9f315c7af00911ca2b8fcb2c3adcbd91ea5cbce5..4f063ce351a348ab19d133ab09bdbbb37501eab5 100644 (file)
@@ -122,7 +122,7 @@ static void test_ds_recurse(int depth, int number, size_t size)
                test_assert_idx(strspn(ps[i], tag) == size - 2, i);
                test_assert_idx(ps[i][size-1] == tag[0], i);
        }
-       test_assert_idx(t_id == t_pop(), depth);
+       test_assert_idx(t_pop(&t_id), depth);
 }
 
 static void test_ds_recursive(int count, int depth)
@@ -165,7 +165,7 @@ enum fatal_test_state fatal_data_stack(int stage)
                undo_ptr = NULL;
                /* t_pop musn't abort, that would cause recursion */
                things_are_messed_up = TRUE;
-               if (t_id != NONEXISTENT_STACK_FRAME_ID && t_pop() != t_id)
+               if (t_id != NONEXISTENT_STACK_FRAME_ID && !t_pop(&t_id))
                        return FATAL_TEST_ABORT; /* abort, things are messed up with us */
                things_are_messed_up = FALSE;
                t_id = NONEXISTENT_STACK_FRAME_ID;
@@ -205,7 +205,7 @@ enum fatal_test_state fatal_data_stack(int stage)
                undo_data = *undo_ptr;
                *undo_ptr = '*';
                /* t_pop will now fail */
-               (void)t_pop();
+               (void)t_pop(&t_id);
                t_id = NONEXISTENT_STACK_FRAME_ID; /* We're FUBAR, mustn't pop next entry */
                return FATAL_TEST_FAILURE;
        }
@@ -218,7 +218,7 @@ enum fatal_test_state fatal_data_stack(int stage)
                undo_data = *undo_ptr;
                *undo_ptr = '*';
                /* t_pop will now fail */
-               (void)t_pop();
+               (void)t_pop(&t_id);
                t_id = NONEXISTENT_STACK_FRAME_ID; /* We're FUBAR, mustn't pop next entry */
                return FATAL_TEST_FAILURE;
        }