Skip to content

Navigation Menu

Sign in

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 407b50f

Browse filesBrowse files
tglsfdctglsfdc
committed
Store GUC data in a memory context, instead of using malloc().
The only real argument for using malloc directly was that we needed the ability to not throw error on OOM; but mcxt.c grew that feature awhile ago. Keeping the data in a memory context improves accountability and debuggability --- for example, without this it's almost impossible to detect memory leaks in the GUC code with anything less costly than valgrind. Moreover, the next patch in this series will add a hash table for GUC lookup, and it'd be pretty silly to be using palloc-dependent hash facilities alongside malloc'd storage of the underlying data. This is a bit invasive though, in particular causing an API break for GUC check hooks that want to modify the GUC's value or use an "extra" data structure. They must now use guc_malloc() and guc_free() instead of malloc() and free(). Failure to change affected code will result in assertion failures or worse; but thanks to recent effort in the mcxt infrastructure, it shouldn't be too hard to diagnose such oversights (at least in assert-enabled builds). One note is that this changes ParseLongOption() to return short-lived palloc'd not malloc'd data. There wasn't any caller for which the previous definition was better. Discussion: https://postgr.es/m/2982579.1662416866@sss.pgh.pa.us
1 parent 9c911ec commit 407b50f
Copy full SHA for 407b50f

File tree

13 files changed

+185
-127
lines changed
Filter options

13 files changed

+185
-127
lines changed

‎src/backend/bootstrap/bootstrap.c

Copy file name to clipboardExpand all lines: src/backend/bootstrap/bootstrap.c
+2-2
Original file line numberDiff line numberDiff line change
@@ -287,8 +287,8 @@ BootstrapModeMain(int argc, char *argv[], bool check_only)
287287
}
288288

289289
SetConfigOption(name, value, PGC_POSTMASTER, PGC_S_ARGV);
290-
free(name);
291-
free(value);
290+
pfree(name);
291+
pfree(value);
292292
break;
293293
}
294294
default:

‎src/backend/commands/tablespace.c

Copy file name to clipboardExpand all lines: src/backend/commands/tablespace.c
+2-2
Original file line numberDiff line numberDiff line change
@@ -1290,8 +1290,8 @@ check_temp_tablespaces(char **newval, void **extra, GucSource source)
12901290
}
12911291

12921292
/* Now prepare an "extra" struct for assign_temp_tablespaces */
1293-
myextra = malloc(offsetof(temp_tablespaces_extra, tblSpcs) +
1294-
numSpcs * sizeof(Oid));
1293+
myextra = guc_malloc(LOG, offsetof(temp_tablespaces_extra, tblSpcs) +
1294+
numSpcs * sizeof(Oid));
12951295
if (!myextra)
12961296
return false;
12971297
myextra->numSpcs = numSpcs;

‎src/backend/commands/variable.c

Copy file name to clipboardExpand all lines: src/backend/commands/variable.c
+17-17
Original file line numberDiff line numberDiff line change
@@ -148,15 +148,15 @@ check_datestyle(char **newval, void **extra, GucSource source)
148148
char *subval;
149149
void *subextra = NULL;
150150

151-
subval = strdup(GetConfigOptionResetString("datestyle"));
151+
subval = guc_strdup(LOG, GetConfigOptionResetString("datestyle"));
152152
if (!subval)
153153
{
154154
ok = false;
155155
break;
156156
}
157157
if (!check_datestyle(&subval, &subextra, source))
158158
{
159-
free(subval);
159+
guc_free(subval);
160160
ok = false;
161161
break;
162162
}
@@ -165,8 +165,8 @@ check_datestyle(char **newval, void **extra, GucSource source)
165165
newDateStyle = myextra[0];
166166
if (!have_order)
167167
newDateOrder = myextra[1];
168-
free(subval);
169-
free(subextra);
168+
guc_free(subval);
169+
guc_free(subextra);
170170
}
171171
else
172172
{
@@ -187,9 +187,9 @@ check_datestyle(char **newval, void **extra, GucSource source)
187187
}
188188

189189
/*
190-
* Prepare the canonical string to return. GUC wants it malloc'd.
190+
* Prepare the canonical string to return. GUC wants it guc_malloc'd.
191191
*/
192-
result = (char *) malloc(32);
192+
result = (char *) guc_malloc(LOG, 32);
193193
if (!result)
194194
return false;
195195

@@ -221,13 +221,13 @@ check_datestyle(char **newval, void **extra, GucSource source)
221221
break;
222222
}
223223

224-
free(*newval);
224+
guc_free(*newval);
225225
*newval = result;
226226

227227
/*
228228
* Set up the "extra" struct actually used by assign_datestyle.
229229
*/
230-
myextra = (int *) malloc(2 * sizeof(int));
230+
myextra = (int *) guc_malloc(LOG, 2 * sizeof(int));
231231
if (!myextra)
232232
return false;
233233
myextra[0] = newDateStyle;
@@ -366,7 +366,7 @@ check_timezone(char **newval, void **extra, GucSource source)
366366
/*
367367
* Pass back data for assign_timezone to use
368368
*/
369-
*extra = malloc(sizeof(pg_tz *));
369+
*extra = guc_malloc(LOG, sizeof(pg_tz *));
370370
if (!*extra)
371371
return false;
372372
*((pg_tz **) *extra) = new_tz;
@@ -439,7 +439,7 @@ check_log_timezone(char **newval, void **extra, GucSource source)
439439
/*
440440
* Pass back data for assign_log_timezone to use
441441
*/
442-
*extra = malloc(sizeof(pg_tz *));
442+
*extra = guc_malloc(LOG, sizeof(pg_tz *));
443443
if (!*extra)
444444
return false;
445445
*((pg_tz **) *extra) = new_tz;
@@ -500,7 +500,7 @@ check_timezone_abbreviations(char **newval, void **extra, GucSource source)
500500
return true;
501501
}
502502

503-
/* OK, load the file and produce a malloc'd TimeZoneAbbrevTable */
503+
/* OK, load the file and produce a guc_malloc'd TimeZoneAbbrevTable */
504504
*extra = load_tzoffsets(*newval);
505505

506506
/* tzparser.c returns NULL on failure, reporting via GUC_check_errmsg */
@@ -647,7 +647,7 @@ check_transaction_deferrable(bool *newval, void **extra, GucSource source)
647647
bool
648648
check_random_seed(double *newval, void **extra, GucSource source)
649649
{
650-
*extra = malloc(sizeof(int));
650+
*extra = guc_malloc(LOG, sizeof(int));
651651
if (!*extra)
652652
return false;
653653
/* Arm the assign only if source of value is an interactive SET */
@@ -735,16 +735,16 @@ check_client_encoding(char **newval, void **extra, GucSource source)
735735
if (strcmp(*newval, canonical_name) != 0 &&
736736
strcmp(*newval, "UNICODE") != 0)
737737
{
738-
free(*newval);
739-
*newval = strdup(canonical_name);
738+
guc_free(*newval);
739+
*newval = guc_strdup(LOG, canonical_name);
740740
if (!*newval)
741741
return false;
742742
}
743743

744744
/*
745745
* Save the encoding's ID in *extra, for use by assign_client_encoding.
746746
*/
747-
*extra = malloc(sizeof(int));
747+
*extra = guc_malloc(LOG, sizeof(int));
748748
if (!*extra)
749749
return false;
750750
*((int *) *extra) = encoding;
@@ -847,7 +847,7 @@ check_session_authorization(char **newval, void **extra, GucSource source)
847847
ReleaseSysCache(roleTup);
848848

849849
/* Set up "extra" struct for assign_session_authorization to use */
850-
myextra = (role_auth_extra *) malloc(sizeof(role_auth_extra));
850+
myextra = (role_auth_extra *) guc_malloc(LOG, sizeof(role_auth_extra));
851851
if (!myextra)
852852
return false;
853853
myextra->roleid = roleid;
@@ -957,7 +957,7 @@ check_role(char **newval, void **extra, GucSource source)
957957
}
958958

959959
/* Set up "extra" struct for assign_role to use */
960-
myextra = (role_auth_extra *) malloc(sizeof(role_auth_extra));
960+
myextra = (role_auth_extra *) guc_malloc(LOG, sizeof(role_auth_extra));
961961
if (!myextra)
962962
return false;
963963
myextra->roleid = roleid;

‎src/backend/postmaster/postmaster.c

Copy file name to clipboardExpand all lines: src/backend/postmaster/postmaster.c
+2-2
Original file line numberDiff line numberDiff line change
@@ -849,8 +849,8 @@ PostmasterMain(int argc, char *argv[])
849849
}
850850

851851
SetConfigOption(name, value, PGC_POSTMASTER, PGC_S_ARGV);
852-
free(name);
853-
free(value);
852+
pfree(name);
853+
pfree(value);
854854
break;
855855
}
856856

‎src/backend/replication/syncrep.c

Copy file name to clipboardExpand all lines: src/backend/replication/syncrep.c
+2-2
Original file line numberDiff line numberDiff line change
@@ -1054,9 +1054,9 @@ check_synchronous_standby_names(char **newval, void **extra, GucSource source)
10541054
return false;
10551055
}
10561056

1057-
/* GUC extra value must be malloc'd, not palloc'd */
1057+
/* GUC extra value must be guc_malloc'd, not palloc'd */
10581058
pconf = (SyncRepConfigData *)
1059-
malloc(syncrep_parse_result->config_size);
1059+
guc_malloc(LOG, syncrep_parse_result->config_size);
10601060
if (pconf == NULL)
10611061
return false;
10621062
memcpy(pconf, syncrep_parse_result, syncrep_parse_result->config_size);

‎src/backend/tcop/postgres.c

Copy file name to clipboardExpand all lines: src/backend/tcop/postgres.c
+2-2
Original file line numberDiff line numberDiff line change
@@ -3871,8 +3871,8 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
38713871
optarg)));
38723872
}
38733873
SetConfigOption(name, value, ctx, gucsource);
3874-
free(name);
3875-
free(value);
3874+
pfree(name);
3875+
pfree(value);
38763876
break;
38773877
}
38783878

‎src/backend/utils/adt/datetime.c

Copy file name to clipboardExpand all lines: src/backend/utils/adt/datetime.c
+4-3
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "utils/builtins.h"
3030
#include "utils/date.h"
3131
#include "utils/datetime.h"
32+
#include "utils/guc.h"
3233
#include "utils/memutils.h"
3334
#include "utils/tzparser.h"
3435

@@ -4782,8 +4783,8 @@ TemporalSimplify(int32 max_precis, Node *node)
47824783
* to create the final array of timezone tokens. The argument array
47834784
* is already sorted in name order.
47844785
*
4785-
* The result is a TimeZoneAbbrevTable (which must be a single malloc'd chunk)
4786-
* or NULL on malloc failure. No other error conditions are defined.
4786+
* The result is a TimeZoneAbbrevTable (which must be a single guc_malloc'd
4787+
* chunk) or NULL on alloc failure. No other error conditions are defined.
47874788
*/
47884789
TimeZoneAbbrevTable *
47894790
ConvertTimeZoneAbbrevs(struct tzEntry *abbrevs, int n)
@@ -4812,7 +4813,7 @@ ConvertTimeZoneAbbrevs(struct tzEntry *abbrevs, int n)
48124813
}
48134814

48144815
/* Alloc the result ... */
4815-
tbl = malloc(tbl_size);
4816+
tbl = guc_malloc(LOG, tbl_size);
48164817
if (!tbl)
48174818
return NULL;
48184819

‎src/backend/utils/cache/ts_cache.c

Copy file name to clipboardExpand all lines: src/backend/utils/cache/ts_cache.c
+3-3
Original file line numberDiff line numberDiff line change
@@ -633,9 +633,9 @@ check_default_text_search_config(char **newval, void **extra, GucSource source)
633633

634634
ReleaseSysCache(tuple);
635635

636-
/* GUC wants it malloc'd not palloc'd */
637-
free(*newval);
638-
*newval = strdup(buf);
636+
/* GUC wants it guc_malloc'd not palloc'd */
637+
guc_free(*newval);
638+
*newval = guc_strdup(LOG, buf);
639639
pfree(buf);
640640
if (!*newval)
641641
return false;

‎src/backend/utils/misc/README

Copy file name to clipboardExpand all lines: src/backend/utils/misc/README
+7-8
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,13 @@ out-of-memory.
5151
This might be used for example to canonicalize the spelling of a string
5252
value, round off a buffer size to the nearest supported value, or replace
5353
a special value such as "-1" with a computed default value. If the
54-
function wishes to replace a string value, it must malloc (not palloc)
55-
the replacement value, and be sure to free() the previous value.
54+
function wishes to replace a string value, it must guc_malloc (not palloc)
55+
the replacement value, and be sure to guc_free() the previous value.
5656

5757
* Derived information, such as the role OID represented by a user name,
58-
can be stored for use by the assign hook. To do this, malloc (not palloc)
58+
can be stored for use by the assign hook. To do this, guc_malloc (not palloc)
5959
storage space for the information, and return its address at *extra.
60-
guc.c will automatically free() this space when the associated GUC setting
60+
guc.c will automatically guc_free() this space when the associated GUC setting
6161
is no longer of interest. *extra is initialized to NULL before call, so
6262
it can be ignored if not needed.
6363

@@ -255,10 +255,9 @@ maintained by GUC.
255255
GUC Memory Handling
256256
-------------------
257257

258-
String variable values are allocated with malloc/strdup, not with the
259-
palloc/pstrdup mechanisms. We would need to keep them in a permanent
260-
context anyway, and malloc gives us more control over handling
261-
out-of-memory failures.
258+
String variable values are allocated with guc_malloc or guc_strdup,
259+
which ensure that the values are kept in a long-lived context, and provide
260+
more control over handling out-of-memory failures than bare palloc.
262261

263262
We allow a string variable's actual value, reset_val, boot_val, and stacked
264263
values to point at the same storage. This makes it slightly harder to free

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.