Re: [PATCH] PGSERVICEFILE as part of a normal connection string

On Sun, Apr 13, 2025 at 07:06:06PM +0900, Ryo Kanbayashi wrote:
> I rebased our patch according to  2c7bd2ba507e.
> https://commitfest.postgresql.org/patch/5387/

Thanks for the new version.

-# for the connection options and their environment variables.
+# for the connection options, servicefile options and their environment variables.

It seems to me that this comment does not need to be changed.

+	{"servicefile", "PGSERVICEFILE", NULL, NULL,
+	"Database-Service-File", "", 64, -1},

Could it be better to have a new field in pg_conn?  This would also
require a free() in freePGconn() and new PQserviceFile() routine.

+                if (strcmp(key, "servicefile") == 0)
+                {
+                    libpq_append_error(errorMessage,
+                                       "nested servicefile specifications not supported in service file \"%s\", line %d",
+                                       serviceFile,
+                                       linenr);
+                    result = 3;
+                    goto exit;
+                }

Perhaps we should add a test for that?  The same is true with
"service", as I am looking at these code paths now.  I'd suggest to
apply double quotes to the parameter name "servicefile" in this error
message, to make clear what this is.

+   # Additionaly encode a colon in servicefile path of Windows 

Typo: Additionally.

+# Backslashes escaped path string for getting collect result at concatenation
+# for Windows environment

Comment is unclear.  But what you mean here is that the conversion is
required to allow the test to work when giving the path to the
connection option, right?
--
Michael