On Sun, Jun 15, 2025 at 09:02:31PM +0900, Ryo Kanbayashi wrote:
> Thanks for review :)
Thanks for the new patch.
While testing the patch, I've bumped into this scenario which feels
incomplete:
- Rely on a default location of the service file, like
$HOME/.pg_service.conf.
- Define a service, with PGSERVICE or a connection parameter.
In this case, :SERVICE shows up some information, not :SERVICEFILE
because it remains empty when building a connection file path if we
don't provide PGSERVICEFILE or servicefile as connection option. It
seems to me that we had better force pg_conn->pgservicefile into a
value in this case, pointing to the value libpq thinks is the default
at the time of resolving the HOME location in pqGetHomeDirectory()?
It seems to me that you should be able to do that at the end of
parseServiceFile(), at least, if we know that the status is a success
(free value if any, assign the new one, and invent an error code path
for the OOM on strdup()).
Defining PGSERVICEFILE or servicefile in a connection string reports
correctly "pgservicefile" in the libpq connection, of course. That's
just for the default location paths.
By the way, could you split the test case for the nested "service"
value in a service file into its own file? This is an existing error
case, and there is no need for the new feature to add this test.
--
Michael
