Re: ALTER SYSTEM and ParseConfigFile()

Stephen Frost <sfrost(at)snowman(dot)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