Hello all,

In several places in the source, there was the same block of code that was
used to deinitialize the log buffer.  There were even two functions that
did this, but they were called only from the code that is in the same
source file (free_tcpcheck_fmt() in src/tcpcheck.c and free_logformat_list()
in src/proxy.c - they were both static functions).

The function free_logformat_list() was moved from the file src/proxy.c to
src/log.c, and a check of the list before freeing the memory was added to
that function.

Best regards,

-- 
Miroslav Zagorac
Senior Developer
From 52ef575d77a47ef7338004fbad31d8199e59c752 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac <mzago...@haproxy.com>
Date: Tue, 30 Jan 2024 03:14:09 +0100
Subject: [PATCH] CLEANUP: log: deinitialization of the log buffer in one
 function

In several places in the source, there was the same block of code that was
used to deinitialize the log buffer.  There were even two functions that
did this, but they were called only from the code that is in the same
source file (free_tcpcheck_fmt() in src/tcpcheck.c and free_logformat_list()
in src/proxy.c - they were both static functions).

The function free_logformat_list() was moved from the file src/proxy.c to
src/log.c, and a check of the list before freeing the memory was added to
that function.
---
 include/haproxy/log.h |  3 +++
 src/fcgi-app.c        | 11 +----------
 src/http_act.c        | 28 ++++------------------------
 src/http_htx.c        | 26 ++++----------------------
 src/http_rules.c      |  9 +--------
 src/log.c             | 16 ++++++++++++++++
 src/proxy.c           | 12 ------------
 src/tcpcheck.c        | 39 +++++++++++++--------------------------
 src/vars.c            |  9 +--------
 9 files changed, 43 insertions(+), 110 deletions(-)

diff --git a/include/haproxy/log.h b/include/haproxy/log.h
index 68b820734..9534eaeb2 100644
--- a/include/haproxy/log.h
+++ b/include/haproxy/log.h
@@ -64,6 +64,9 @@ void syslog_fd_handler(int fd);
 int init_log_buffers(void);
 void deinit_log_buffers(void);
 
+/* Deinitialize log buffers used for syslog messages */
+void free_logformat_list(struct list *fmt);
+
 /* build a log line for the session and an optional stream */
 int sess_build_logline(struct session *sess, struct stream *s, char *dst, size_t maxsize, struct list *list_format);
 
diff --git a/src/fcgi-app.c b/src/fcgi-app.c
index 00562f80d..dcd7fd224 100644
--- a/src/fcgi-app.c
+++ b/src/fcgi-app.c
@@ -134,16 +134,7 @@ static void fcgi_release_rule(struct fcgi_rule *rule)
 	if (!rule)
 		return;
 
-	if (!LIST_ISEMPTY(&rule->value)) {
-		struct logformat_node *lf, *lfb;
-
-		list_for_each_entry_safe(lf, lfb, &rule->value, list) {
-			LIST_DELETE(&lf->list);
-			release_sample_expr(lf->expr);
-			free(lf->arg);
-			free(lf);
-		}
-	}
+	free_logformat_list(&rule->value);
 	/* ->cond and ->name are not owned by the rule */
 	free(rule);
 }
diff --git a/src/http_act.c b/src/http_act.c
index 260fe1d67..ebc2bcc68 100644
--- a/src/http_act.c
+++ b/src/http_act.c
@@ -46,17 +46,10 @@
  */
 static void release_http_action(struct act_rule *rule)
 {
-	struct logformat_node *lf, *lfb;
-
 	istfree(&rule->arg.http.str);
 	if (rule->arg.http.re)
 		regex_free(rule->arg.http.re);
-	list_for_each_entry_safe(lf, lfb, &rule->arg.http.fmt, list) {
-		LIST_DELETE(&lf->list);
-		release_sample_expr(lf->expr);
-		free(lf->arg);
-		free(lf);
-	}
+	free_logformat_list(&rule->arg.http.fmt);
 }
 
 /* Release memory allocated by HTTP actions relying on an http reply. Concretly,
@@ -1901,23 +1894,10 @@ static enum act_return http_action_set_map(struct act_rule *rule, struct proxy *
 /* Release memory allocated by an http map/acl action. */
 static void release_http_map(struct act_rule *rule)
 {
-	struct logformat_node *lf, *lfb;
-
 	free(rule->arg.map.ref);
-	list_for_each_entry_safe(lf, lfb, &rule->arg.map.key, list) {
-		LIST_DELETE(&lf->list);
-		release_sample_expr(lf->expr);
-		free(lf->arg);
-		free(lf);
-	}
-	if (rule->action == 1) {
-		list_for_each_entry_safe(lf, lfb, &rule->arg.map.value, list) {
-			LIST_DELETE(&lf->list);
-			release_sample_expr(lf->expr);
-			free(lf->arg);
-			free(lf);
-		}
-	}
+	free_logformat_list(&rule->arg.map.key);
+	if (rule->action == 1)
+		free_logformat_list(&rule->arg.map.value);
 }
 
 /* Parse a "add-acl", "del-acl", "set-map" or "del-map" actions. It takes one or
diff --git a/src/http_htx.c b/src/http_htx.c
index 004d3439a..954473db3 100644
--- a/src/http_htx.c
+++ b/src/http_htx.c
@@ -1117,7 +1117,6 @@ int http_str_to_htx(struct buffer *buf, struct ist raw, char **errmsg)
 
 void release_http_reply(struct http_reply *http_reply)
 {
-	struct logformat_node *lf, *lfb;
 	struct http_reply_hdr *hdr, *hdrb;
 
 	if (!http_reply)
@@ -1126,12 +1125,7 @@ void release_http_reply(struct http_reply *http_reply)
 	ha_free(&http_reply->ctype);
 	list_for_each_entry_safe(hdr, hdrb, &http_reply->hdrs, list) {
 		LIST_DELETE(&hdr->list);
-		list_for_each_entry_safe(lf, lfb, &hdr->value, list) {
-			LIST_DELETE(&lf->list);
-			release_sample_expr(lf->expr);
-			free(lf->arg);
-			free(lf);
-		}
+		free_logformat_list(&hdr->value);
 		istfree(&hdr->name);
 		free(hdr);
 	}
@@ -1141,14 +1135,8 @@ void release_http_reply(struct http_reply *http_reply)
 	}
 	else if (http_reply->type == HTTP_REPLY_RAW)
 		chunk_destroy(&http_reply->body.obj);
-	else if (http_reply->type == HTTP_REPLY_LOGFMT) {
-		list_for_each_entry_safe(lf, lfb, &http_reply->body.fmt, list) {
-			LIST_DELETE(&lf->list);
-			release_sample_expr(lf->expr);
-			free(lf->arg);
-			free(lf);
-		}
-	}
+	else if (http_reply->type == HTTP_REPLY_LOGFMT)
+		free_logformat_list(&http_reply->body.fmt);
 	free(http_reply);
 }
 
@@ -1497,7 +1485,6 @@ int http_check_http_reply(struct http_reply *reply, struct proxy *px, char **err
 struct http_reply *http_parse_http_reply(const char **args, int *orig_arg, struct proxy *px,
 					 int default_status, char **errmsg)
 {
-	struct logformat_node *lf, *lfb;
 	struct http_reply *reply = NULL;
 	struct http_reply_hdr *hdr, *hdrb;
 	struct stat stat;
@@ -1778,12 +1765,7 @@ struct http_reply *http_parse_http_reply(const char **args, int *orig_arg, struc
 				   px->conf.args.file, px->conf.args.line);
 			list_for_each_entry_safe(hdr, hdrb, &reply->hdrs, list) {
 				LIST_DELETE(&hdr->list);
-				list_for_each_entry_safe(lf, lfb, &hdr->value, list) {
-					LIST_DELETE(&lf->list);
-					release_sample_expr(lf->expr);
-					free(lf->arg);
-					free(lf);
-				}
+				free_logformat_list(&hdr->value);
 				istfree(&hdr->name);
 				free(hdr);
 			}
diff --git a/src/http_rules.c b/src/http_rules.c
index 192f0c7d3..9fde30aca 100644
--- a/src/http_rules.c
+++ b/src/http_rules.c
@@ -320,17 +320,10 @@ struct act_rule *parse_http_after_res_cond(const char **args, const char *file,
 /* completely free redirect rule */
 void http_free_redirect_rule(struct redirect_rule *rdr)
 {
-	struct logformat_node *lf, *lfb;
-
 	free_acl_cond(rdr->cond);
 	free(rdr->rdr_str);
 	free(rdr->cookie_str);
-	list_for_each_entry_safe(lf, lfb, &rdr->rdr_fmt, list) {
-		LIST_DELETE(&lf->list);
-		release_sample_expr(lf->expr);
-		free(lf->arg);
-		free(lf);
-	}
+	free_logformat_list(&rdr->rdr_fmt);
 	free(rdr);
 }
 
diff --git a/src/log.c b/src/log.c
index 010ace9dc..a71f20606 100644
--- a/src/log.c
+++ b/src/log.c
@@ -2575,6 +2575,22 @@ void deinit_log_forward()
 	}
 }
 
+/* Releases memory allocated for a log-format string */
+void free_logformat_list(struct list *fmt)
+{
+	struct logformat_node *lf, *lfb;
+
+	if ((fmt == NULL) || LIST_ISEMPTY(fmt))
+		return;
+
+	list_for_each_entry_safe(lf, lfb, fmt, list) {
+		LIST_DELETE(&lf->list);
+		release_sample_expr(lf->expr);
+		free(lf->arg);
+		free(lf);
+	}
+}
+
 /* Builds a log line in <dst> based on <list_format>, and stops before reaching
  * <maxsize> characters. Returns the size of the output string in characters,
  * not counting the trailing zero which is always added if the resulting size
diff --git a/src/proxy.c b/src/proxy.c
index 6451cbb8a..4436ccfa1 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -177,18 +177,6 @@ void free_stick_rules(struct list *rules)
 	}
 }
 
-static void free_logformat_list(struct list *lfs)
-{
-	struct logformat_node *lf, *lfb;
-
-	list_for_each_entry_safe(lf, lfb, lfs, list) {
-		LIST_DELETE(&lf->list);
-		release_sample_expr(lf->expr);
-		free(lf->arg);
-		free(lf);
-	}
-}
-
 void free_server_rules(struct list *srules)
 {
 	struct server_rule *srule, *sruleb;
diff --git a/src/tcpcheck.c b/src/tcpcheck.c
index 050015d1f..6efca5bf5 100644
--- a/src/tcpcheck.c
+++ b/src/tcpcheck.c
@@ -75,26 +75,13 @@ DECLARE_POOL(pool_head_tcpcheck_rule, "tcpcheck_rule", sizeof(struct tcpcheck_ru
 /**************************************************************************/
 /*************** Init/deinit tcp-check rules and ruleset ******************/
 /**************************************************************************/
-/* Releases memory allocated for a log-format string */
-static void free_tcpcheck_fmt(struct list *fmt)
-{
-	struct logformat_node *lf, *lfb;
-
-	list_for_each_entry_safe(lf, lfb, fmt, list) {
-		LIST_DELETE(&lf->list);
-		release_sample_expr(lf->expr);
-		free(lf->arg);
-		free(lf);
-	}
-}
-
 /* Releases memory allocated for an HTTP header used in a tcp-check send rule */
 void free_tcpcheck_http_hdr(struct tcpcheck_http_hdr *hdr)
 {
 	if (!hdr)
 		return;
 
-	free_tcpcheck_fmt(&hdr->value);
+	free_logformat_list(&hdr->value);
 	istfree(&hdr->name);
 	free(hdr);
 }
@@ -131,28 +118,28 @@ void free_tcpcheck(struct tcpcheck_rule *rule, int in_pool)
 			break;
 		case TCPCHK_SEND_STRING_LF:
 		case TCPCHK_SEND_BINARY_LF:
-			free_tcpcheck_fmt(&rule->send.fmt);
+			free_logformat_list(&rule->send.fmt);
 			break;
 		case TCPCHK_SEND_HTTP:
 			free(rule->send.http.meth.str.area);
 			if (!(rule->send.http.flags & TCPCHK_SND_HTTP_FL_URI_FMT))
 				istfree(&rule->send.http.uri);
 			else
-				free_tcpcheck_fmt(&rule->send.http.uri_fmt);
+				free_logformat_list(&rule->send.http.uri_fmt);
 			istfree(&rule->send.http.vsn);
 			free_tcpcheck_http_hdrs(&rule->send.http.hdrs);
 			if (!(rule->send.http.flags & TCPCHK_SND_HTTP_FL_BODY_FMT))
 				istfree(&rule->send.http.body);
 			else
-				free_tcpcheck_fmt(&rule->send.http.body_fmt);
+				free_logformat_list(&rule->send.http.body_fmt);
 			break;
 		case TCPCHK_SEND_UNDEF:
 			break;
 		}
 		break;
 	case TCPCHK_ACT_EXPECT:
-		free_tcpcheck_fmt(&rule->expect.onerror_fmt);
-		free_tcpcheck_fmt(&rule->expect.onsuccess_fmt);
+		free_logformat_list(&rule->expect.onerror_fmt);
+		free_logformat_list(&rule->expect.onsuccess_fmt);
 		release_sample_expr(rule->expect.status_expr);
 		switch (rule->expect.type) {
 		case TCPCHK_EXPECT_HTTP_STATUS:
@@ -172,20 +159,20 @@ void free_tcpcheck(struct tcpcheck_rule *rule, int in_pool)
 		case TCPCHK_EXPECT_STRING_LF:
 		case TCPCHK_EXPECT_BINARY_LF:
 		case TCPCHK_EXPECT_HTTP_BODY_LF:
-			free_tcpcheck_fmt(&rule->expect.fmt);
+			free_logformat_list(&rule->expect.fmt);
 			break;
 		case TCPCHK_EXPECT_HTTP_HEADER:
 			if (rule->expect.flags & TCPCHK_EXPT_FL_HTTP_HNAME_REG)
 				regex_free(rule->expect.hdr.name_re);
 			else if (rule->expect.flags & TCPCHK_EXPT_FL_HTTP_HNAME_FMT)
-				free_tcpcheck_fmt(&rule->expect.hdr.name_fmt);
+				free_logformat_list(&rule->expect.hdr.name_fmt);
 			else
 				istfree(&rule->expect.hdr.name);
 
 			if (rule->expect.flags & TCPCHK_EXPT_FL_HTTP_HVAL_REG)
 				regex_free(rule->expect.hdr.value_re);
 			else if (rule->expect.flags & TCPCHK_EXPT_FL_HTTP_HVAL_FMT)
-				free_tcpcheck_fmt(&rule->expect.hdr.value_fmt);
+				free_logformat_list(&rule->expect.hdr.value_fmt);
 			else if (!(rule->expect.flags & TCPCHK_EXPT_FL_HTTP_HVAL_NONE))
 				istfree(&rule->expect.hdr.value);
 			break;
@@ -3513,7 +3500,7 @@ void tcpcheck_overwrite_send_http_rule(struct tcpcheck_rule *old, struct tcpchec
 		if (!(old->send.http.flags & TCPCHK_SND_HTTP_FL_URI_FMT))
 			istfree(&old->send.http.uri);
 		else
-			free_tcpcheck_fmt(&old->send.http.uri_fmt);
+			free_logformat_list(&old->send.http.uri_fmt);
 		old->send.http.flags &= ~TCPCHK_SND_HTTP_FL_URI_FMT;
 		old->send.http.uri = new->send.http.uri;
 		new->send.http.uri = IST_NULL;
@@ -3522,7 +3509,7 @@ void tcpcheck_overwrite_send_http_rule(struct tcpcheck_rule *old, struct tcpchec
 		if (!(old->send.http.flags & TCPCHK_SND_HTTP_FL_URI_FMT))
 			istfree(&old->send.http.uri);
 		else
-			free_tcpcheck_fmt(&old->send.http.uri_fmt);
+			free_logformat_list(&old->send.http.uri_fmt);
 		old->send.http.flags |= TCPCHK_SND_HTTP_FL_URI_FMT;
 		LIST_INIT(&old->send.http.uri_fmt);
 		list_for_each_entry_safe(lf, lfb, &new->send.http.uri_fmt, list) {
@@ -3549,7 +3536,7 @@ void tcpcheck_overwrite_send_http_rule(struct tcpcheck_rule *old, struct tcpchec
 		if (!(old->send.http.flags & TCPCHK_SND_HTTP_FL_BODY_FMT))
 			istfree(&old->send.http.body);
 		else
-			free_tcpcheck_fmt(&old->send.http.body_fmt);
+			free_logformat_list(&old->send.http.body_fmt);
 		old->send.http.flags &= ~TCPCHK_SND_HTTP_FL_BODY_FMT;
 		old->send.http.body = new->send.http.body;
 		new->send.http.body = IST_NULL;
@@ -3558,7 +3545,7 @@ void tcpcheck_overwrite_send_http_rule(struct tcpcheck_rule *old, struct tcpchec
 		if (!(old->send.http.flags & TCPCHK_SND_HTTP_FL_BODY_FMT))
 			istfree(&old->send.http.body);
 		else
-			free_tcpcheck_fmt(&old->send.http.body_fmt);
+			free_logformat_list(&old->send.http.body_fmt);
 		old->send.http.flags |= TCPCHK_SND_HTTP_FL_BODY_FMT;
 		LIST_INIT(&old->send.http.body_fmt);
 		list_for_each_entry_safe(lf, lfb, &new->send.http.body_fmt, list) {
diff --git a/src/vars.c b/src/vars.c
index 0962a1c9c..05e79f44a 100644
--- a/src/vars.c
+++ b/src/vars.c
@@ -838,14 +838,7 @@ static enum act_return action_clear(struct act_rule *rule, struct proxy *px,
 
 static void release_store_rule(struct act_rule *rule)
 {
-	struct logformat_node *lf, *lfb;
-
-	list_for_each_entry_safe(lf, lfb, &rule->arg.vars.fmt, list) {
-		LIST_DELETE(&lf->list);
-		release_sample_expr(lf->expr);
-		free(lf->arg);
-		free(lf);
-	}
+	free_logformat_list(&rule->arg.vars.fmt);
 
 	release_sample_expr(rule->arg.vars.expr);
 }
-- 
2.42.0

Reply via email to