From: Willy Tarreau <w@1wt.eu>
Date: Mon, 24 Jan 2022 10:51:43 +0000 (+0100)
Subject: MINOR: pools: partially uninline pool_free()
X-Git-Tag: v2.6-dev1~90
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=15c322c41362b3221c19202556b077d87ebd5080;p=thirdparty%2Fhaproxy.git

MINOR: pools: partially uninline pool_free()

The pool_free() function has become a bit big over time due to the
extra consistency checks. It used to remain inline only to deal
cleanly with the NULL pointer free that's quite present on some
structures (e.g. in stream_free()).

Here we're splitting the function in two:
  - __pool_free() does the inner block without the pointer test and
    becomes a function ;

  - pool_free() is now a macro that only checks the pointer and calls
    __pool_free() if needed.

The use of a macro versus an inline function is only motivated by an
easier intrumentation of the code later.

With this change, the code size reduces by ~1%, which means that at
this point all pool_free() call places used to represent more than
1% of the total code size.
---

diff --git a/include/haproxy/pool.h b/include/haproxy/pool.h
index 61428a884f..d3074084f4 100644
--- a/include/haproxy/pool.h
+++ b/include/haproxy/pool.h
@@ -97,6 +97,7 @@ void create_pool_callback(struct pool_head **ptr, char *name, unsigned int size)
 void *pool_destroy(struct pool_head *pool);
 void pool_destroy_all(void);
 int mem_should_fail(const struct pool_head *pool);
+void __pool_free(struct pool_head *pool, void *ptr);
 
 
 #ifdef CONFIG_HAP_POOLS
@@ -345,26 +346,15 @@ static inline void *pool_zalloc(struct pool_head *pool)
 }
 
 /*
- * Puts a memory area back to the corresponding pool.
- * Items are chained directly through a pointer that
- * is written in the beginning of the memory area, so
- * there's no need for any carrier cell. This implies
- * that each memory area is at least as big as one
- * pointer. Just like with the libc's free(), nothing
- * is done if <ptr> is NULL.
+ * Puts a memory area back to the corresponding pool. Just like with the libc's
+ * free(), <ptr> may be NULL.
  */
-static inline void pool_free(struct pool_head *pool, void *ptr)
-{
-        if (likely(ptr != NULL)) {
-		/* we'll get late corruption if we refill to the wrong pool or double-free */
-		POOL_DEBUG_CHECK_MARK(pool, ptr);
-
-		if (unlikely(mem_poison_byte >= 0))
-			memset(ptr, mem_poison_byte, pool->size);
-
-		pool_put_to_cache(pool, ptr);
-	}
-}
+#define pool_free(pool, ptr)				\
+	do {						\
+		typeof(ptr) __ptr = (ptr);		\
+		if (likely((__ptr) != NULL))		\
+			__pool_free(pool, __ptr);	\
+	} while (0)
 
 #endif /* _HAPROXY_POOL_H */
 
diff --git a/src/pool.c b/src/pool.c
index 06089c3c59..21b48af4d0 100644
--- a/src/pool.c
+++ b/src/pool.c
@@ -588,6 +588,21 @@ void pool_gc(struct pool_head *pool_ctx)
 
 #endif /* CONFIG_HAP_POOLS */
 
+/*
+ * Puts a memory area back to the corresponding pool. <ptr> be valid. Using
+ * pool_free() is preferred.
+ */
+void __pool_free(struct pool_head *pool, void *ptr)
+{
+	/* we'll get late corruption if we refill to the wrong pool or double-free */
+	POOL_DEBUG_CHECK_MARK(pool, ptr);
+
+	if (unlikely(mem_poison_byte >= 0))
+		memset(ptr, mem_poison_byte, pool->size);
+
+	pool_put_to_cache(pool, ptr);
+}
+
 
 #ifdef DEBUG_UAF