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

On Wed, Jul 09, 2025 at 04:31:31PM +0900, Michael Paquier wrote:
> Attached is a rebased version of the rest, with the recent stanza
> related to fef6da9e9c87 taken into account. 0002 still has a change
> that should be in 0001: I have not really touched the structure of the
> two remaining patches yet.

+ /* If service was found successfully, set servicefile option if not already set */
+ if (*group_found && result == 0)
+ {
+ for (i = 0; options[i].keyword; i++)
+ {
+ if (strcmp(options[i].keyword, "servicefile") != 0)
+ continue;
+
+ if (options[i].val != NULL)
+ break;
+
+ options[i].val = strdup(serviceFile);
+ if (options[i].val == NULL)
+ {
+ libpq_append_error(errorMessage, "out of memory");
+ return 3;
+ }
+ break;

There was a bug here: if the new value cannot be strdup'd, we would
miss the fclose() of the exit path, so this cannot return directly.
It is possible to set the status to a new value instead, then break.

After that, I have applied a few cosmetic tweaks here and there, and
attached is what I have staged for commit, minus proper commit
messages. The new TAP tests have some WIN32-specific things, and I
won't be able to look at the buildfarm if I were to apply things
today, so this will have to wait until the beginning of next week.
The CI is happy with it, so at least we are one checkbox down.
--
Michael