Hi, I've been trying to run Postgis regress tests under Clang sanitizers and one of the issues I'm facing is the constant stream of errors during the `configure` step coming from calls to `pg_config`.
Example: ``` $ pg_config --cc clang ================================================================= ==14521==ERROR: LeakSanitizer: detected memory leaks Direct leak of 368 byte(s) in 1 object(s) allocated from: #0 0x55de20d161d9 in malloc (/usr/bin/pg_config+0xf81d9) [...] SUMMARY: AddressSanitizer: 2610 byte(s) leaked in 47 allocation(s). ``` The leaked memory is part of the `configdata` array which isn't freed before exiting. It doesn't have any long term impact but it's annoying. A similar thing happens in the `pg_config` SQL function. Since the memory will be released at the end of the transaction, releasing it is optional but I've done it anyway. I'm attaching a the patch with the changes. Greetings, Greetings, -- Raúl Marín Rodríguez carto.com
From 0d35d7a21b87554df7ef27b70dcf7e4ea512699f Mon Sep 17 00:00:00 2001 From: Raul Marin <git@rmr.ninja> Date: Wed, 14 Nov 2018 09:08:50 +0100 Subject: [PATCH] pg_config: Avoid leaking configdata --- src/backend/utils/misc/pg_config.c | 1 + src/bin/pg_config/pg_config.c | 4 ++++ src/common/config_info.c | 15 ++++++++++++++- src/include/common/config_info.h | 2 ++ 4 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/backend/utils/misc/pg_config.c b/src/backend/utils/misc/pg_config.c index aa434bc3ab..62d1aea287 100644 --- a/src/backend/utils/misc/pg_config.c +++ b/src/backend/utils/misc/pg_config.c @@ -78,6 +78,7 @@ pg_config(PG_FUNCTION_ARGS) tuple = BuildTupleFromCStrings(attinmeta, values); tuplestore_puttuple(tupstore, tuple); } + free_configdata(configdata, configdata_len); /* * no longer need the tuple descriptor reference created by diff --git a/src/bin/pg_config/pg_config.c b/src/bin/pg_config/pg_config.c index a341b756de..c53a802422 100644 --- a/src/bin/pg_config/pg_config.c +++ b/src/bin/pg_config/pg_config.c @@ -160,6 +160,7 @@ main(int argc, char **argv) { for (i = 0; i < configdata_len; i++) printf("%s = %s\n", configdata[i].name, configdata[i].setting); + free_configdata(configdata, configdata_len); exit(0); } @@ -180,9 +181,12 @@ main(int argc, char **argv) fprintf(stderr, _("%s: invalid argument: %s\n"), progname, argv[i]); advice(); + free_configdata(configdata, configdata_len); exit(1); } } + free_configdata(configdata, configdata_len); + return 0; } diff --git a/src/common/config_info.c b/src/common/config_info.c index 55e688e656..f8da71c598 100644 --- a/src/common/config_info.c +++ b/src/common/config_info.c @@ -27,7 +27,7 @@ * get_configdata(const char *my_exec_path, size_t *configdata_len) * * Get configure-time constants. The caller is responsible - * for pfreeing the result. + * for pfreeing the result [free_configdata] */ ConfigData * get_configdata(const char *my_exec_path, size_t *configdata_len) @@ -203,3 +203,16 @@ get_configdata(const char *my_exec_path, size_t *configdata_len) return configdata; } + + +void +free_configdata(ConfigData *configdata, size_t configdata_len) +{ + int i; + for (i = 0; i < configdata_len; i++) + { + pfree(configdata[i].name); + pfree(configdata[i].setting); + } + pfree(configdata); +} diff --git a/src/include/common/config_info.h b/src/include/common/config_info.h index 72014a915a..26f85e86a9 100644 --- a/src/include/common/config_info.h +++ b/src/include/common/config_info.h @@ -18,4 +18,6 @@ typedef struct ConfigData extern ConfigData *get_configdata(const char *my_exec_path, size_t *configdata_len); +extern void free_configdata(ConfigData *configdata, size_t configdata_len); + #endif /* COMMON_CONFIG_INFO_H */ -- 2.19.1