diff --git a/src/lib-master/master-service.c b/src/lib-master/master-service.c index edbb2cb1ca..9294f059f3 100644 --- a/src/lib-master/master-service.c +++ b/src/lib-master/master-service.c @@ -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"); } } diff --git a/src/lib-test/test-common.c b/src/lib-test/test-common.c index c8568014e4..b026492d52 100644 --- a/src/lib-test/test-common.c +++ b/src/lib-test/test-common.c @@ -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); } diff --git a/src/lib/data-stack.c b/src/lib/data-stack.c index 8e1b53bffa..7e07150230 100644 --- a/src/lib/data-stack.c +++ b/src/lib/data-stack.c @@ -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 diff --git a/src/lib/data-stack.h b/src/lib/data-stack.h index 14aeb81385..6867c08a9a 100644 --- a/src/lib/data-stack.h +++ b/src/lib/data-stack.h @@ -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. diff --git a/src/lib/ioloop.c b/src/lib/ioloop.c index 91da3830ae..e006da0d8c 100644 --- a/src/lib/ioloop.c +++ b/src/lib/ioloop.c @@ -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); } diff --git a/src/lib/test-data-stack.c b/src/lib/test-data-stack.c index 9f315c7af0..4f063ce351 100644 --- a/src/lib/test-data-stack.c +++ b/src/lib/test-data-stack.c @@ -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; }