From a469356268f463d24b9d0e71e0cfa5826ac02a5c Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Wed, 13 Aug 2025 11:52:59 +0200 Subject: [PATCH] MEDIUM: server: introduce srv_alloc()/srv_free() to alloc/free a server It happens that we free servers at various places in the code, both on error paths and at runtime thanks to the "server delete" feature. In order to switch to an aligned struct, we'll need to change the calloc() and free() calls. Let's first spot them and switch them to srv_alloc() and srv_free() instead of using calloc() and either free() or ha_free(). An easy trap to fall into is that some of them are default-server entries. The new srv_free() function also resets the pointer like ha_free() does. This was done by running the following coccinelle script all over the code: @@ struct server *srv; @@ ( - free(srv) + srv_free(&srv) | - ha_free(&srv) + srv_free(&srv) ) @@ struct server *srv; expression e1; expression e2; @@ ( - srv = malloc(e1) + srv = srv_alloc() | - srv = calloc(e1, e2) + srv = srv_alloc() ) This is marked medium because despite spotting all call places, we can never rule out the possibility that some out-of-tree patches would allocate their own servers and continue to use the old API... at their own risk. --- include/haproxy/server.h | 17 +++++++++++++++++ src/cli.c | 2 +- src/proxy.c | 12 ++++++------ src/server.c | 8 ++++---- 4 files changed, 28 insertions(+), 11 deletions(-) diff --git a/include/haproxy/server.h b/include/haproxy/server.h index 19f0017fd..dc0e263c8 100644 --- a/include/haproxy/server.h +++ b/include/haproxy/server.h @@ -179,6 +179,23 @@ void srv_set_dyncookie(struct server *s); int srv_check_reuse_ws(struct server *srv); const struct mux_ops *srv_get_ws_proto(struct server *srv); +/* allocate a new server and return it (or NULL on failure). The structure is + * zeroed. + */ +static inline struct server *srv_alloc(void) +{ + struct server *srv; + + srv = calloc(1, sizeof(*srv)); + return srv; +} + +/* free a previously allocated server an nullifies the pointer */ +static inline void srv_free(struct server **srv_ptr) +{ + ha_free(srv_ptr); +} + /* increase the number of cumulated streams on the designated server */ static inline void srv_inc_sess_ctr(struct server *s) { diff --git a/src/cli.c b/src/cli.c index 9895b819e..09a356c00 100644 --- a/src/cli.c +++ b/src/cli.c @@ -3726,7 +3726,7 @@ error: list_for_each_entry(child, &proc_list, list) { free((char *)child->srv->conf.file); /* cast because of const char * */ free(child->srv->id); - ha_free(&child->srv); + srv_free(&child->srv); } free(msg); diff --git a/src/proxy.c b/src/proxy.c index caa33be2f..2bcd87f6f 100644 --- a/src/proxy.c +++ b/src/proxy.c @@ -388,7 +388,7 @@ void deinit_proxy(struct proxy *p) */ if (p->defsrv) { srv_free_params(p->defsrv); - ha_free(&p->defsrv); + srv_free(&p->defsrv); } if (p->lbprm.proxy_deinit) @@ -1560,7 +1560,7 @@ void proxy_free_defaults(struct proxy *defproxy) if (defproxy->defsrv) ha_free((char **)&defproxy->defsrv->conf.file); ha_free(&defproxy->defbe.name); - ha_free(&defproxy->defsrv); + srv_free(&defproxy->defsrv); h = defproxy->req_cap; while (h) { @@ -1708,7 +1708,7 @@ int setup_new_proxy(struct proxy *px, const char *name, unsigned int cap, char * if (name) memprintf(errmsg, "proxy '%s': %s", name, *errmsg); - ha_free(&px->defsrv); + srv_free(&px->defsrv); ha_free(&px->id); counters_fe_shared_drop(&px->fe_counters.shared); counters_be_shared_drop(&px->be_counters.shared); @@ -1740,7 +1740,7 @@ struct proxy *alloc_new_proxy(const char *name, unsigned int cap, char **errmsg) * quitting. */ if (curproxy) - free(curproxy->defsrv); + srv_free(&curproxy->defsrv); free(curproxy); return NULL; } @@ -1802,7 +1802,7 @@ static int proxy_defproxy_cpy(struct proxy *curproxy, const struct proxy *defpro * none allocated yet in the current proxy so we have * to allocate and pre-initialize it right now. */ - curproxy->defsrv = calloc(1, sizeof(*curproxy->defsrv)); + curproxy->defsrv = srv_alloc(); if (!curproxy->defsrv) { memprintf(errmsg, "proxy '%s': out of memory allocating default-server", curproxy->id); return 1; @@ -2833,7 +2833,7 @@ static int post_section_px_cleanup() if (curproxy->defsrv) { ha_free((char **)&curproxy->defsrv->conf.file); - ha_free(&curproxy->defsrv); + srv_free(&curproxy->defsrv); } } return 0; diff --git a/src/server.c b/src/server.c index c299d202b..3d270e8c2 100644 --- a/src/server.c +++ b/src/server.c @@ -3042,7 +3042,7 @@ struct server *new_server(struct proxy *proxy) { struct server *srv; - srv = calloc(1, sizeof *srv); + srv = srv_alloc(); if (!srv) return NULL; @@ -3202,7 +3202,7 @@ struct server *srv_drop(struct server *srv) EXTRA_COUNTERS_FREE(srv->extra_counters); - ha_free(&srv); + srv_free(&srv); end: return next; @@ -3354,7 +3354,7 @@ static int _srv_parse_tmpl_init(struct server *srv, struct proxy *px) free_check(&newsrv->check); MT_LIST_DELETE(&newsrv->global_list); } - free(newsrv); + srv_free(&newsrv); return i - srv->tmpl_info.nb_low; } @@ -3712,7 +3712,7 @@ static int _srv_parse_init(struct server **srv, char **args, int *cur_arg, */ newsrv = curproxy->defsrv; if (!newsrv) { - newsrv = calloc(1, sizeof(*newsrv)); + newsrv = srv_alloc(); if (!newsrv) { ha_alert("out of memory.\n"); err_code |= ERR_ALERT | ERR_ABORT;