Re: ALTER SYSTEM and ParseConfigFile() - Mailing list pgsql-hackers

From Tom Lane
Subject Re: ALTER SYSTEM and ParseConfigFile()
Date
Msg-id 11786.1431108133@sss.pgh.pa.us
Whole thread Raw
In response to ALTER SYSTEM and ParseConfigFile()  (Stephen Frost <sfrost@snowman.net>)
Responses Re: ALTER SYSTEM and ParseConfigFile()  (Stephen Frost <sfrost@snowman.net>)
List pgsql-hackers
Stephen Frost <sfrost@snowman.net> writes:
> Greetings,
>   While working through the pg_file_settings patch, I came across this
>   comment above ParseConfigFp() (which is called by ParseConfigFile()):

> src/backend/utils/misc/guc-file.l:603
> ------------------------------------------------------
>  * Output parameters:
>  *  head_p, tail_p: head and tail of linked list of name/value pairs
>  *
>  * *head_p and *tail_p must be initialized to NULL before calling the outer
>  * recursion level.  On exit, they contain a list of name-value pairs read
>  * from the input file(s).
> ------------------------------------------------------

>   However, in 65d6e4c, ProcessConfigFile(), which isn't part of the
>   recursion, was updated with a second call to ParseConfigFile (for the
>   PG_AUTOCONF_FILENAME file), passing in the head and tail values which
>   had been set by the first call.

>   I'm a bit nervous that there might be an issue here due to how flex
>   errors are handled and the recursion, though it might also be fine
>   (but then why comment about it?).

>   In any case, either the comment needs to be changed, or we should be
>   passing clean NULL variables to ParseConfigFile and then combining the
>   results in ProcessConfigFile().

I think the code is OK, but yeah, this comment should be changed to
reflect the idea that the function will append entries to an existing
list of name/value pairs (and thus, that head_p/tail_p are not output
but in/out parameters).
        regards, tom lane



pgsql-hackers by date:

Previous
From: Stephen Frost
Date:
Subject: ALTER SYSTEM and ParseConfigFile()
Next
From: Andres Freund
Date:
Subject: Re: INSERT ... ON CONFLICT UPDATE/IGNORE 4.0