Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

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

 Sign up
Appearance settings

Commitbf007a2

Browse files
committed
Clean up assorted issues in ALTER SYSTEM coding.
Fix unsafe use of a non-volatile variable in PG_TRY/PG_CATCH inAlterSystemSetConfigFile(). While at it, clean up a bundle of otherinfelicities and outright bugs, including corner-case-incorrect linked listmanipulation, a poorly designed and worse documented parse-and-validatefunction (which even included some randomly chosen hard-wired substitutesfor the specified elevel in one code path ... wtf?), direct use of open()instead of fd.c's facilities, inadequate checking of write()'s returnvalue, and generally poorly written commentary.
1 parentfd49612 commitbf007a2

File tree

2 files changed

+352
-411
lines changed

2 files changed

+352
-411
lines changed

‎src/backend/utils/misc/guc-file.l

Lines changed: 51 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -118,11 +118,11 @@ ProcessConfigFile(GucContext context)
118118
boolerror = false;
119119
boolapply = false;
120120
intelevel;
121+
const char *ConfFileWithError;
121122
ConfigVariable *item,
122-
*head,
123-
*tail;
123+
*head,
124+
*tail;
124125
inti;
125-
char*ErrorConfFile = ConfigFileName;
126126
127127
/*
128128
* Config files are processed on startup (by the postmaster only)
@@ -138,6 +138,7 @@ ProcessConfigFile(GucContext context)
138138
elevel = IsUnderPostmaster ? DEBUG2 : LOG;
139139
140140
/* Parse the main config file into a list of option names and values */
141+
ConfFileWithError = ConfigFileName;
141142
head = tail = NULL;
142143
143144
if (!ParseConfigFile(ConfigFileName, NULL, true, 0, elevel, &head, &tail))
@@ -160,25 +161,23 @@ ProcessConfigFile(GucContext context)
160161
{
161162
/* Syntax error(s) detected in the file, so bail out */
162163
error =true;
163-
ErrorConfFile = PG_AUTOCONF_FILENAME;
164+
ConfFileWithError = PG_AUTOCONF_FILENAME;
164165
goto cleanup_list;
165166
}
166167
}
167168
else
168169
{
169-
ConfigVariable *prev =NULL;
170-
171170
/*
172-
* Pick up only the data_directory if DataDir is not set, which
173-
* means that the configuration file is read for the first time and
174-
* PG_AUTOCONF_FILENAME file cannot be read yet. In this case,
175-
* we shouldn't pick any settings except the data_directory
176-
* from postgresql.conf because they might be overwritten
177-
* with the settings in PG_AUTOCONF_FILENAME file which will be
178-
* read later. OTOH, since it's ensured that data_directory doesn't
179-
* exist in PG_AUTOCONF_FILENAME file, it will never be overwritten
180-
* later.
171+
* If DataDir is not set, the PG_AUTOCONF_FILENAME file cannot be
172+
* read. In this case, we don't want to accept any settings but
173+
* data_directory from postgresql.conf, because they might be
174+
* overwritten with settings in the PG_AUTOCONF_FILENAME file which
175+
* will be read later. OTOH, since data_directory isn't allowed in the
176+
* PG_AUTOCONF_FILENAME file, it will never be overwritten later.
181177
*/
178+
ConfigVariable *prev =NULL;
179+
180+
/* Prune all items except "data_directory" from the list */
182181
for (item = head; item;)
183182
{
184183
ConfigVariable *ptr = item;
@@ -189,26 +188,20 @@ ProcessConfigFile(GucContext context)
189188
if (prev ==NULL)
190189
head = ptr->next;
191190
else
192-
{
193191
prev->next = ptr->next;
194-
/*
195-
* On removing last item in list, we need to update tail
196-
* to ensure that list will be maintianed.
197-
*/
198-
if (prev->next ==NULL)
199-
tail = prev;
200-
}
192+
if (ptr->next ==NULL)
193+
tail = prev;
201194
FreeConfigVariable(ptr);
202195
}
203196
else
204197
prev = ptr;
205198
}
206199

207200
/*
208-
* Quick exit if data_directory is not present inlist.
201+
* Quick exit if data_directory is not present infile.
209202
*
210-
*Don't remember when we last successfully loaded the config file in
211-
*this case becausethattimewill be set soon by subsequentload of
203+
*We need not do any further processing, in particular we don't set
204+
*PgReloadTime;that will be set soon by subsequentfull loading of
212205
* the config file.
213206
*/
214207
if (head ==NULL)
@@ -263,7 +256,7 @@ ProcessConfigFile(GucContext context)
263256
item->name,
264257
item->filename, item->sourceline)));
265258
error =true;
266-
ErrorConfFile = item->filename;
259+
ConfFileWithError = item->filename;
267260
}
268261
}
269262

@@ -392,7 +385,7 @@ ProcessConfigFile(GucContext context)
392385
elseif (scres ==0)
393386
{
394387
error =true;
395-
ErrorConfFile = item->filename;
388+
ConfFileWithError = item->filename;
396389
}
397390
/* else no error but variable's active value was not changed */
398391

@@ -421,23 +414,23 @@ ProcessConfigFile(GucContext context)
421414
ereport(ERROR,
422415
(errcode(ERRCODE_CONFIG_FILE_ERROR),
423416
errmsg("configuration file\"%s\" contains errors",
424-
ErrorConfFile)));
417+
ConfFileWithError)));
425418
elseif (apply)
426419
ereport(elevel,
427420
(errcode(ERRCODE_CONFIG_FILE_ERROR),
428421
errmsg("configuration file\"%s\" contains errors; unaffected changes were applied",
429-
ErrorConfFile)));
422+
ConfFileWithError)));
430423
else
431424
ereport(elevel,
432425
(errcode(ERRCODE_CONFIG_FILE_ERROR),
433426
errmsg("configuration file\"%s\" contains errors; no changes were applied",
434-
ErrorConfFile)));
427+
ConfFileWithError)));
435428
}
436429

437430
/*
438431
* Calling FreeConfigVariables() any earlier than this can cause problems,
439-
* becauseErrorConfFile could be pointing to a string that will be freed
440-
* here.
432+
* becauseConfFileWithError could be pointing to a string that will be
433+
*freedhere.
441434
*/
442435
FreeConfigVariables(head);
443436
}
@@ -477,8 +470,11 @@ AbsoluteConfigLocation(const char *location, const char *calling_file)
477470
* Read and parse a single configuration file. This function recurses
478471
* to handle "include" directives.
479472
*
480-
* See ParseConfigFp for details. This one merely adds opening the
481-
* file rather than working from a caller-supplied file descriptor,
473+
* If "strict" is true, treat failure to open the config file as an error,
474+
* otherwise just skip the file.
475+
*
476+
* See ParseConfigFp for further details. This one merely adds opening the
477+
* config file rather than working from a caller-supplied file descriptor,
482478
* and absolute-ifying the path name if necessary.
483479
*/
484480
bool
@@ -516,12 +512,13 @@ ParseConfigFile(const char *config_file, const char *calling_file, bool strict,
516512
errmsg("could not open configuration file\"%s\": %m",
517513
abs_path)));
518514
OK =false;
519-
goto cleanup;
520515
}
521-
522-
ereport(LOG,
523-
(errmsg("skipping missing configuration file\"%s\"",
524-
abs_path)));
516+
else
517+
{
518+
ereport(LOG,
519+
(errmsg("skipping missing configuration file\"%s\"",
520+
abs_path)));
521+
}
525522
goto cleanup;
526523
}
527524

@@ -616,9 +613,7 @@ ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel,
616613
{
617614
char *opt_name =NULL;
618615
char *opt_value =NULL;
619-
ConfigVariable *item,
620-
*cur_item =NULL,
621-
*prev_item =NULL;
616+
ConfigVariable *item;
622617

623618
if (token == GUC_EOL)/* empty or comment line */
624619
continue;
@@ -701,41 +696,13 @@ ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel,
701696
}
702697
else
703698
{
704-
/*
705-
* ordinary variable, append to list. For multiple items of
706-
* same parameter, retain only which comes later.
707-
*/
699+
/* ordinary variable, append to list */
708700
item =palloc(sizeof *item);
709701
item->name = opt_name;
710702
item->value = opt_value;
711703
item->filename =pstrdup(config_file);
712704
item->sourceline = ConfigFileLineno-1;
713705
item->next =NULL;
714-
715-
/* Remove the existing item of same parameter from the list */
716-
for (cur_item = *head_p; cur_item; prev_item = cur_item,
717-
cur_item = cur_item->next)
718-
{
719-
if (strcmp(item->name, cur_item->name) ==0)
720-
{
721-
if (prev_item ==NULL)
722-
*head_p = cur_item->next;
723-
else
724-
{
725-
prev_item->next = cur_item->next;
726-
/*
727-
* On removing last item in list, we need to update tail
728-
* to ensure that list will be maintianed.
729-
*/
730-
if (prev_item->next ==NULL)
731-
*tail_p = prev_item;
732-
}
733-
734-
FreeConfigVariable(cur_item);
735-
break;
736-
}
737-
}
738-
739706
if (*head_p ==NULL)
740707
*head_p = item;
741708
else
@@ -911,21 +878,6 @@ cleanup:
911878
return status;
912879
}
913880

914-
/*
915-
* Free a ConfigVariable
916-
*/
917-
staticvoid
918-
FreeConfigVariable(ConfigVariable *item)
919-
{
920-
if (item !=NULL)
921-
{
922-
pfree(item->name);
923-
pfree(item->value);
924-
pfree(item->filename);
925-
pfree(item);
926-
}
927-
}
928-
929881
/*
930882
* Free a list of ConfigVariables, including the names and the values
931883
*/
@@ -944,6 +896,18 @@ FreeConfigVariables(ConfigVariable *list)
944896
}
945897
}
946898

899+
/*
900+
* Free a single ConfigVariable
901+
*/
902+
staticvoid
903+
FreeConfigVariable(ConfigVariable *item)
904+
{
905+
pfree(item->name);
906+
pfree(item->value);
907+
pfree(item->filename);
908+
pfree(item);
909+
}
910+
947911

948912
/*
949913
*scanstr

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp