Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 7704a1a

Browse files
committed
Be more wary about NULL values for GUC string variables.
get_explain_guc_options() crashed if a string GUC marked GUC_EXPLAIN has a NULL boot_val. Nosing around found a couple of other places that seemed insufficiently cautious about NULL string values, although those are likely unreachable in practice. Add some commentary defining the expectations for NULL values of string variables, in hopes of forestalling future additions of more such bugs. Xing Guo, Aleksander Alekseev, Tom Lane Discussion: https://postgr.es/m/CACpMh+AyDx5YUpPaAgzVwC1d8zfOL4JoD-uyFDnNSa1z0EsDQQ@mail.gmail.com
1 parent 4b14e18 commit 7704a1a

File tree

2 files changed

+23
-3
lines changed

2 files changed

+23
-3
lines changed

src/backend/utils/misc/guc.c

+13-3
Original file line numberDiff line numberDiff line change
@@ -1473,7 +1473,9 @@ check_GUC_init(struct config_generic *gconf)
14731473
{
14741474
struct config_string *conf = (struct config_string *) gconf;
14751475

1476-
if (*conf->variable != NULL && strcmp(*conf->variable, conf->boot_val) != 0)
1476+
if (*conf->variable != NULL &&
1477+
(conf->boot_val == NULL ||
1478+
strcmp(*conf->variable, conf->boot_val) != 0))
14771479
{
14781480
elog(LOG, "GUC (PGC_STRING) %s, boot_val=%s, C-var=%s",
14791481
conf->gen.name, conf->boot_val ? conf->boot_val : "<null>", *conf->variable);
@@ -5255,7 +5257,14 @@ get_explain_guc_options(int *num)
52555257
{
52565258
struct config_string *lconf = (struct config_string *) conf;
52575259

5258-
modified = (strcmp(lconf->boot_val, *(lconf->variable)) != 0);
5260+
if (lconf->boot_val == NULL &&
5261+
*lconf->variable == NULL)
5262+
modified = false;
5263+
else if (lconf->boot_val == NULL ||
5264+
*lconf->variable == NULL)
5265+
modified = true;
5266+
else
5267+
modified = (strcmp(lconf->boot_val, *(lconf->variable)) != 0);
52595268
}
52605269
break;
52615270

@@ -5482,7 +5491,8 @@ write_one_nondefault_variable(FILE *fp, struct config_generic *gconf)
54825491
{
54835492
struct config_string *conf = (struct config_string *) gconf;
54845493

5485-
fprintf(fp, "%s", *conf->variable);
5494+
if (*conf->variable)
5495+
fprintf(fp, "%s", *conf->variable);
54865496
}
54875497
break;
54885498

src/include/utils/guc_tables.h

+10
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,16 @@ struct config_real
240240
void *reset_extra;
241241
};
242242

243+
/*
244+
* A note about string GUCs: the boot_val is allowed to be NULL, which leads
245+
* to the reset_val and the actual variable value (*variable) also being NULL.
246+
* However, there is no way to set a NULL value subsequently using
247+
* set_config_option or any other GUC API. Also, GUC APIs such as SHOW will
248+
* display a NULL value as an empty string. Callers that choose to use a NULL
249+
* boot_val should overwrite the setting later in startup, or else be careful
250+
* that NULL doesn't have semantics that are visibly different from an empty
251+
* string.
252+
*/
243253
struct config_string
244254
{
245255
struct config_generic gen;

0 commit comments

Comments
 (0)