lib: Changed t_pop() API to make it a bit more like free()

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");
 	}
 }

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);
 }

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

src/lib/data-stack.h

@@ -39,18 +39,17 @@ 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
    frame. However, you should always prefer to use T_BEGIN { ... } T_END below.
 */
 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.

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);
 	}

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;
 	}