From 24a5e42db60a00e616e6a73041b7a562588b3d19 Mon Sep 17 00:00:00 2001 From: Miroslav Zagorac 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 68b820734f19..9534eaeb2336 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 00562f80ddec..dcd7fd224f28 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 260fe1d67146..ebc2bcc68e1a 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 004d3439aac7..954473db3742 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 192f0c7d3768..9fde30acab30 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 010ace9dcb90..a71f206060bc 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 based on , and stops before reaching * 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 6451cbb8a826..4436ccfa1071 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 050015d1f591..6efca5bf54b1 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 0962a1c9c519..05e79f44a8d2 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); }