From: Tom Lane <tgl@sss.pgh.pa.us>
To: Daniel Gustafsson <daniel@yesql.se>
cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Stale references to guc.c in comments/tests
In-reply-to: <6B50C70C-8C1F-4F9A-A7C0-EEAFCC032406@yesql.se>
References: <6B50C70C-8C1F-4F9A-A7C0-EEAFCC032406@yesql.se>
Comments: In-reply-to Daniel Gustafsson <daniel@yesql.se>
	message dated "Fri, 24 Feb 2023 14:15:55 +0100"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <837371.1677251944.1@sss.pgh.pa.us>
Date: Fri, 24 Feb 2023 10:19:04 -0500
Message-ID: <837372.1677251944@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/837372.1677251944%40sss.pgh.pa.us>
Precedence: bulk

Daniel Gustafsson <daniel@yesql.se> writes:
> I happened to notice that there were a few references to guc.c regarding
> variables, which with the recent refactoring in 0a20ff54f have become stale.
> Attached is a trivial patch to instead point to guc_tables.c.

Hmm, I think you may have done an overenthusiastic replacement here.
I agree with changes like this:

-extern char *role_string;		/* in guc.c */
+extern char *role_string;		/* in guc_tables.c */

because clearly that variable is now declared in guc_tables.c
and guc.c knows nothing of it explicitly.  However, a lot of
these places are really talking about the behavior of the GUC
mechanisms as a whole, and so a pointer to guc_tables.c doesn't
seem very on-point to me --- I find it hard to attribute behavior
to a static table.  Here for instance:

@@ -3041,7 +3041,7 @@ pg_get_functiondef(PG_FUNCTION_ARGS)
                  *
                  * Variables that are not so marked should just be emitted as
                  * simple string literals.  If the variable is not known to
-                 * guc.c, we'll do that; this makes it unsafe to use
+                 * guc_tables.c, we'll do that; this makes it unsafe to use
                  * GUC_LIST_QUOTE for extension variables.
                  */
                 if (GetConfigOptionFlags(configitem, true) & GUC_LIST_QUOTE)

An extension's GUC is by definition not known in guc_tables.c, so I think
this change is losing the point of the text.  What it's really describing
is a variable that hasn't been entered into the dynamic tables maintained
by guc.c.

Perhaps you could use "the GUC mechanisms" in these places, but it's a bit
longer than "guc.c".  Leaving such references alone seems OK too.

			regards, tom lane