From 8c4927098e994e26441db64e77eba11b9757b915 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w@1wt.eu>
Date: Sat, 1 Jan 2022 17:10:50 +0100
Subject: [PATCH] CLEANUP: pools: get rid of the POOL_LINK macro

The POOL_LINK macro is now only used for debugging, and it still requires
ifdefs around, which needlessly complicates the code. Let's replace it
and the calling code with a new pair of macros: POOL_DEBUG_SET_MARK()
and POOL_DEBUG_CHECK_MARK(), that respectively store and check the pool
pointer in the extra location at the end of the pool. This removes 4
pairs of ifdefs in the middle of the code.
---
 include/haproxy/pool-t.h | 14 ------------
 include/haproxy/pool.h   | 49 +++++++++++++++++++++++++++++++---------
 src/pool.c               |  4 +---
 3 files changed, 39 insertions(+), 28 deletions(-)

diff --git a/include/haproxy/pool-t.h b/include/haproxy/pool-t.h
index d476f31d4..c1d2613f3 100644
--- a/include/haproxy/pool-t.h
+++ b/include/haproxy/pool-t.h
@@ -28,20 +28,6 @@
 #define MEM_F_SHARED	0x1
 #define MEM_F_EXACT	0x2
 
-/* By default, free objects are linked by a pointer stored at the beginning of
- * the memory area. When DEBUG_MEMORY_POOLS is set, the allocated area is
- * inflated by the size of a pointer so that the link is placed at the end
- * of the objects. Hence free objects in pools remain intact. In addition,
- * this location is used to keep a pointer to the pool the object was
- * allocated from, and verify it's freed into the appropriate one.
- */
-#ifdef DEBUG_MEMORY_POOLS
-#define POOL_EXTRA (sizeof(void *))
-#define POOL_LINK(pool, item) (void **)(((char *)(item)) + ((pool)->size))
-#else
-#define POOL_EXTRA (0)
-#endif
-
 /* A special pointer for the pool's free_list that indicates someone is
  * currently manipulating it. Serves as a short-lived lock.
  */
diff --git a/include/haproxy/pool.h b/include/haproxy/pool.h
index d8407b19e..896fb6142 100644
--- a/include/haproxy/pool.h
+++ b/include/haproxy/pool.h
@@ -45,6 +45,39 @@
 	static struct pool_head *(ptr) __read_mostly; \
 	REGISTER_POOL(&ptr, name, size)
 
+/* By default, free objects are linked by a pointer stored at the beginning of
+ * the memory area. When DEBUG_MEMORY_POOLS is set, the allocated area is
+ * inflated by the size of a pointer so that the link is placed at the end
+ * of the objects. Hence free objects in pools remain intact. In addition,
+ * this location is used to keep a pointer to the pool the object was
+ * allocated from, and verify it's freed into the appropriate one.
+ */
+#ifdef DEBUG_MEMORY_POOLS
+
+# define POOL_EXTRA (sizeof(void *))
+# define POOL_DEBUG_SET_MARK(pool, item)				\
+	do {								\
+		typeof(pool) __p = (pool);				\
+		typeof(item) __i = (item);				\
+		*(typeof(pool)*)(((char *)__i) + __p->size) = __p;	\
+	} while (0)
+
+# define POOL_DEBUG_CHECK_MARK(pool, item)				\
+	do {								\
+		typeof(pool) __p = (pool);				\
+		typeof(item) __i = (item);				\
+		if (*(typeof(pool)*)(((char *)__i) + __p->size) != __p)	\
+			ABORT_NOW();					\
+	} while (0)
+
+#else // DEBUG_MEMORY_POOLS
+
+# define POOL_EXTRA (0)
+# define POOL_DEBUG_SET_MARK(pool, item)   do { } while (0)
+# define POOL_DEBUG_CHECK_MARK(pool, item) do { } while (0)
+
+#endif // DEBUG_MEMORY_POOLS
+
 /* poison each newly allocated area with this byte if >= 0 */
 extern int mem_poison_byte;
 
@@ -135,11 +168,8 @@ static inline void *pool_get_from_shared_cache(struct pool_head *pool)
 	_HA_ATOMIC_STORE(&pool->free_list, *(void **)ret);
 	_HA_ATOMIC_INC(&pool->used);
 
-#ifdef DEBUG_MEMORY_POOLS
 	/* keep track of where the element was allocated from */
-	*POOL_LINK(pool, ret) = (void *)pool;
-#endif
-
+	POOL_DEBUG_SET_MARK(pool, ret);
  out:
 	__ha_barrier_atomic_store();
 	return ret;
@@ -197,10 +227,9 @@ static inline void *pool_get_from_cache(struct pool_head *pool)
 	pool_cache_count--;
 	LIST_DELETE(&item->by_pool);
 	LIST_DELETE(&item->by_lru);
-#ifdef DEBUG_MEMORY_POOLS
+
 	/* keep track of where the element was allocated from */
-	*POOL_LINK(pool, item) = (void *)pool;
-#endif
+	POOL_DEBUG_SET_MARK(pool, item);
 	return item;
 }
 
@@ -282,11 +311,9 @@ static inline void *pool_zalloc(struct pool_head *pool)
 static inline void pool_free(struct pool_head *pool, void *ptr)
 {
         if (likely(ptr != NULL)) {
-#ifdef DEBUG_MEMORY_POOLS
 		/* we'll get late corruption if we refill to the wrong pool or double-free */
-		if (*POOL_LINK(pool, ptr) != (void *)pool)
-			ABORT_NOW();
-#endif
+		POOL_DEBUG_CHECK_MARK(pool, ptr);
+
 		if (unlikely(mem_poison_byte >= 0))
 			memset(ptr, mem_poison_byte, pool->size);
 
diff --git a/src/pool.c b/src/pool.c
index 7d83a953f..efebb4ad4 100644
--- a/src/pool.c
+++ b/src/pool.c
@@ -282,10 +282,8 @@ void *pool_alloc_nocache(struct pool_head *pool)
 	swrate_add_scaled(&pool->needed_avg, POOL_AVG_SAMPLES, pool->used, POOL_AVG_SAMPLES/4);
 	_HA_ATOMIC_INC(&pool->used);
 
-#ifdef DEBUG_MEMORY_POOLS
 	/* keep track of where the element was allocated from */
-	*POOL_LINK(pool, ptr) = (void *)pool;
-#endif
+	POOL_DEBUG_SET_MARK(pool, ptr);
 	return ptr;
 }