Re: ALTER SYSTEM and ParseConfigFile()

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