On Sun, Jul 06, 2025 at 09:13:19AM -0700, Noah Misch wrote: > This new PQservice() function from commit 4b99fed75 came up in the annual > exports.txt diff. The standard in libpq has been to not clutter the API with > new functions that simply retrieve one PQconninfoOption value. PQconninfo() > provides access to all those values in a generic way. What do you think of > making psql use PQconninfo() for this, then removing PQservice()? The rest of > the commit (adding the struct field, necessary for PQconninfo() to include the > value) looks good.
Sure, I was not aware of such a policy. Relying on PQconninfoOption is less efficient because we would need to look through the whole set of options when looking for the service name, and this needs one extra allocation as PQconninfoFree() frees the values allocated. With two callers perhaps this inefficiency is OK to live with anyway. What do you think about the attached, then? -- Michael
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 9fcd2db83265..c839634d4233 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -4480,6 +4480,7 @@ SyncVariables(void) { char vbuf[32]; const char *server_version; + char *service_name; /* get stuff from connection */ pset.encoding = PQclientEncoding(pset.db); @@ -4489,12 +4490,16 @@ SyncVariables(void) setFmtEncoding(pset.encoding); SetVariable(pset.vars, "DBNAME", PQdb(pset.db)); - SetVariable(pset.vars, "SERVICE", PQservice(pset.db)); SetVariable(pset.vars, "USER", PQuser(pset.db)); SetVariable(pset.vars, "HOST", PQhost(pset.db)); SetVariable(pset.vars, "PORT", PQport(pset.db)); SetVariable(pset.vars, "ENCODING", pg_encoding_to_char(pset.encoding)); + service_name = get_service_name(); + SetVariable(pset.vars, "SERVICE", service_name); + if (service_name) + pg_free(service_name); + /* this bit should match connection_warnings(): */ /* Try to get full text form of version, might include "devel" etc */ server_version = PQparameterStatus(pset.db, "server_version"); diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c index d2c0a49c46c0..3593c0468310 100644 --- a/src/bin/psql/common.c +++ b/src/bin/psql/common.c @@ -2531,6 +2531,41 @@ session_username(void) return PQuser(pset.db); } +/* + * Return the service name of the current connection. + * + * The caller is responsible for freeing the result value allocated. + */ +char * +get_service_name(void) +{ + PQconninfoOption *opts; + PQconninfoOption *serviceopt = NULL; + char *res = NULL; + + if (pset.db == NULL) + return NULL; + + opts = PQconninfo(pset.db); + if (opts == NULL) + return NULL; + + for (PQconninfoOption *opt = opts; opt->keyword != NULL; ++opt) + { + if (strcmp(opt->keyword, "service") == 0) + { + serviceopt = opt; + continue; + } + } + + /* Take a copy of the value, as it is freed by PQconninfoFree(). */ + if (serviceopt && serviceopt->val != NULL) + res = pg_strdup(serviceopt->val); + PQconninfoFree(opts); + + return res; +} /* expand_tilde * diff --git a/src/bin/psql/common.h b/src/bin/psql/common.h index 7f1a23de1e82..261199df3187 100644 --- a/src/bin/psql/common.h +++ b/src/bin/psql/common.h @@ -39,6 +39,7 @@ extern bool SendQuery(const char *query); extern bool is_superuser(void); extern bool standard_strings(void); extern const char *session_username(void); +extern char *get_service_name(void); extern void expand_tilde(char **filename); extern void clean_extended_state(void); diff --git a/src/bin/psql/prompt.c b/src/bin/psql/prompt.c index 3aa7d2d06c80..cdae9b681500 100644 --- a/src/bin/psql/prompt.c +++ b/src/bin/psql/prompt.c @@ -169,8 +169,15 @@ get_prompt(promptStatus_t status, ConditionalStack cstack) break; /* service name */ case 's': - if (pset.db && PQservice(pset.db)) - strlcpy(buf, PQservice(pset.db), sizeof(buf)); + { + char *service_name = get_service_name(); + + if (service_name) + { + strlcpy(buf, service_name, sizeof(buf)); + pg_free(service_name); + } + } break; /* backend pid */ case 'p': diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt index 0625cf39e9af..dbbae642d769 100644 --- a/src/interfaces/libpq/exports.txt +++ b/src/interfaces/libpq/exports.txt @@ -205,9 +205,8 @@ PQcancelFinish 202 PQsocketPoll 203 PQsetChunkedRowsMode 204 PQgetCurrentTimeUSec 205 -PQservice 206 -PQsetAuthDataHook 207 -PQgetAuthDataHook 208 -PQdefaultAuthDataHook 209 -PQfullProtocolVersion 210 -appendPQExpBufferVA 211 +PQsetAuthDataHook 206 +PQgetAuthDataHook 207 +PQdefaultAuthDataHook 208 +PQfullProtocolVersion 209 +appendPQExpBufferVA 210 diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 51a9c4165845..09eb79812ac6 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -7461,14 +7461,6 @@ PQdb(const PGconn *conn) return conn->dbName; } -char * -PQservice(const PGconn *conn) -{ - if (!conn) - return NULL; - return conn->pgservice; -} - char * PQuser(const PGconn *conn) { diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h index 7d3a9df6fd55..af8004f952a5 100644 --- a/src/interfaces/libpq/libpq-fe.h +++ b/src/interfaces/libpq/libpq-fe.h @@ -400,7 +400,6 @@ extern int PQrequestCancel(PGconn *conn); /* Accessor functions for PGconn objects */ extern char *PQdb(const PGconn *conn); -extern char *PQservice(const PGconn *conn); extern char *PQuser(const PGconn *conn); extern char *PQpass(const PGconn *conn); extern char *PQhost(const PGconn *conn); diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 298c4b38ef90..b2c2cf9eac83 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -2740,26 +2740,6 @@ char *PQport(const PGconn *conn); </listitem> </varlistentry> - <varlistentry id="libpq-PQservice"> - <term><function>PQservice</function><indexterm><primary>PQservice</primary></indexterm></term> - - <listitem> - <para> - Returns the service of the active connection. - -<synopsis> -char *PQservice(const PGconn *conn); -</synopsis> - </para> - - <para> - <xref linkend="libpq-PQservice"/> returns <symbol>NULL</symbol> if the - <parameter>conn</parameter> argument is <symbol>NULL</symbol>. - Otherwise, if there was no service provided, it returns an empty string. - </para> - </listitem> - </varlistentry> - <varlistentry id="libpq-PQtty"> <term><function>PQtty</function><indexterm><primary>PQtty</primary></indexterm></term>
signature.asc
Description: PGP signature