On 15.10.24 14:00, Alexander Lakhin wrote:
I also wonder, if other places touched by 5d2e1cc11 need corrections too. I played with PG_COLOR=always PG_COLORS="error=01;31" .../initdband it looks like this free() call in pg_logging_init(): char *colors = strdup(pg_colors_env); if (colors) { ... while ((token = strsep(&colors, ":"))) { ... } free(colors); } gets null in colors.
Yes, this is indeed incorrect. We need to keep a separate pointer to the start of the string to free later. This matches the example on the strsep man page (https://man.freebsd.org/cgi/man.cgi?strsep(3)). Patch attached.
From 1b842bfdc2b303264134687f3ec21fd3e53a0c82 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pe...@eisentraut.org> Date: Wed, 16 Oct 2024 09:37:54 +0200 Subject: [PATCH] Fix memory leaks from incorrect strsep() uses Commit 5d2e1cc117b introduced some strsep() uses, but it did the memory management wrong in some cases. We need to keep a separate pointer to the allocate memory so that we can free it later, because strsep() advances the pointer we pass to it, and it at the end it will be NULL, so any free() calls won't do anything. (This fixes two of the four places changed in commit 5d2e1cc117b. The other two don't have this problem.) Reported-by: Alexander Lakhin <exclus...@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/79692bf9-17d3-41e6-b9c9-fc8c39442...@eisentraut.org --- src/common/logging.c | 3 ++- src/test/regress/pg_regress.c | 7 +++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/common/logging.c b/src/common/logging.c index aedd1ae2d8c..3cf119090a5 100644 --- a/src/common/logging.c +++ b/src/common/logging.c @@ -120,8 +120,9 @@ pg_logging_init(const char *argv0) if (colors) { char *token; + char *cp = colors; - while ((token = strsep(&colors, ":"))) + while ((token = strsep(&cp, ":"))) { char *e = strchr(token, '='); diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index 5157629b1cc..6c188954b14 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -233,14 +233,17 @@ free_stringlist(_stringlist **listhead) static void split_to_stringlist(const char *s, const char *delim, _stringlist **listhead) { - char *sc = pg_strdup(s); char *token; + char *sc; + char *tofree; + + tofree = sc = pg_strdup(s); while ((token = strsep(&sc, delim))) { add_stringlist_item(listhead, token); } - free(sc); + free(tofree); } /* -- 2.47.0
