Обсуждение: BUG #6216: Calling PQconnectdbParams from C++ with a char**
The following bug has been logged online: Bug reference: 6216 Logged by: Lionel Elie Mamane Email address: lionel@mamane.lu PostgreSQL version: 9.1.0 Operating system: Debian GNU/Linux Description: Calling PQconnectdbParams from C++ with a char** Details: In C++, a "char**" value is not convertible to a "const char**" value, because that is not safe (see http://www.parashift.com/c++-faq-lite/const-correctness.html#faq-18.17). This means one cannot call libpq's PQconnectdbParams and friends passing them a "char**" value for keywords and/or values, as these arguments are declared "const char**". So please apply this patch, which declares them "char const* const*" instead, which is: - equivalent to "const char * const*"; use that one if you prefer - still true, since libpq won't write to they keyword/values arrays - allows to pass a "char**" to them and be recognised by a C++ compiler as OK without a cast. This patch is licensed under a "do whatever you want with it" license. Thanks in advance. --- postgresql-9.1-9.1.0.orig/src/interfaces/libpq/fe-connect.c +++ postgresql-9.1-9.1.0/src/interfaces/libpq/fe-connect.c @@ -291,8 +291,8 @@ static void freePGconn(PGconn *conn); static void closePGconn(PGconn *conn); static PQconninfoOption *conninfo_parse(const char *conninfo, PQExpBuffer errorMessage, bool use_defaults); -static PQconninfoOption *conninfo_array_parse(const char **keywords, - const char **values, PQExpBuffer errorMessage, +static PQconninfoOption *conninfo_array_parse(char const* const*keywords, + char const* const*values, PQExpBuffer errorMessage, bool use_defaults, int expand_dbname); static char *conninfo_getval(PQconninfoOption *connOptions, const char *keyword); @@ -362,8 +362,8 @@ pgthreadlock_t pg_g_threadlock = default * call succeeded. */ PGconn * -PQconnectdbParams(const char **keywords, - const char **values, +PQconnectdbParams(char const* const*keywords, + char const* const*values, int expand_dbname) { PGconn *conn = PQconnectStartParams(keywords, values, expand_dbname); @@ -381,8 +381,8 @@ PQconnectdbParams(const char **keywords, * check server status, accepting parameters identical to PQconnectdbParams */ PGPing -PQpingParams(const char **keywords, - const char **values, +PQpingParams(char const* const*keywords, + char const* const*values, int expand_dbname) { PGconn *conn = PQconnectStartParams(keywords, values, expand_dbname); @@ -464,8 +464,8 @@ PQping(const char *conninfo) * See PQconnectPoll for more info. */ PGconn * -PQconnectStartParams(const char **keywords, - const char **values, +PQconnectStartParams(char const* const*keywords, + char const* const*values, int expand_dbname) { PGconn *conn; @@ -4249,7 +4249,7 @@ conninfo_parse(const char *conninfo, PQE * keywords will take precedence, however. */ static PQconninfoOption * -conninfo_array_parse(const char **keywords, const char **values, +conninfo_array_parse(char const* const*keywords, char const* const*values, PQExpBuffer errorMessage, bool use_defaults, int expand_dbname) { --- postgresql-9.1-9.1.0.orig/src/interfaces/libpq/libpq-fe.h +++ postgresql-9.1-9.1.0/src/interfaces/libpq/libpq-fe.h @@ -235,14 +235,14 @@ typedef struct pgresAttDesc /* make a new client connection to the backend */ /* Asynchronous (non-blocking) */ extern PGconn *PQconnectStart(const char *conninfo); -extern PGconn *PQconnectStartParams(const char **keywords, - const char **values, int expand_dbname); +extern PGconn *PQconnectStartParams(char const* const*keywords, + char const* const*values, int expand_dbname); extern PostgresPollingStatusType PQconnectPoll(PGconn *conn); /* Synchronous (blocking) */ extern PGconn *PQconnectdb(const char *conninfo); -extern PGconn *PQconnectdbParams(const char **keywords, - const char **values, int expand_dbname); +extern PGconn *PQconnectdbParams(char const* const*keywords, + char const* const*values, int expand_dbname); extern PGconn *PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions, const char *pgtty, const char *dbName, @@ -413,8 +413,8 @@ extern int PQsetnonblocking(PGconn *conn extern int PQisnonblocking(const PGconn *conn); extern int PQisthreadsafe(void); extern PGPing PQping(const char *conninfo); -extern PGPing PQpingParams(const char **keywords, - const char **values, int expand_dbname); +extern PGPing PQpingParams(char const* const*keywords, + char const* const*values, int expand_dbname); /* Force the write buffer to be written (or at least try) */ extern int PQflush(PGconn *conn);
On Tue, Sep 20, 2011 at 11:05 AM, Lionel Elie Mamane <lionel@mamane.lu> wro= te: > > The following bug has been logged online: > > Bug reference: =A0 =A0 =A06216 > Logged by: =A0 =A0 =A0 =A0 =A0Lionel Elie Mamane > Email address: =A0 =A0 =A0lionel@mamane.lu > PostgreSQL version: 9.1.0 > Operating system: =A0 Debian GNU/Linux > Description: =A0 =A0 =A0 =A0Calling PQconnectdbParams from C++ with a cha= r** > Details: > > In C++, a "char**" value is not convertible to a "const char**" value, > because that is not safe (see > http://www.parashift.com/c++-faq-lite/const-correctness.html#faq-18.17). > > This means one cannot call libpq's PQconnectdbParams and friends passing > them a "char**" value for keywords and/or values, as these arguments are > declared "const char**". > > So please apply this patch, which declares them "char const* const*" > instead, which is: > =A0- equivalent to "const char * const*"; use that one if you prefer > =A0- still true, since libpq won't write to they keyword/values arrays > =A0- allows to pass a "char**" to them and be recognised by a C++ compile= r as > OK without a cast. > > This patch is licensed under a "do whatever you want with it" license. > > Thanks in advance. > > > --- postgresql-9.1-9.1.0.orig/src/interfaces/libpq/fe-connect.c > +++ postgresql-9.1-9.1.0/src/interfaces/libpq/fe-connect.c > @@ -291,8 +291,8 @@ static void freePGconn(PGconn *conn); > =A0static void closePGconn(PGconn *conn); > =A0static PQconninfoOption *conninfo_parse(const char *conninfo, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 PQExpBuffer errorMess= age, bool use_defaults); > -static PQconninfoOption *conninfo_array_parse(const char **keywords, > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0const char **values, PQExpBuffer errorMessage, > +static PQconninfoOption *conninfo_array_parse(char const* const*keywords, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0char const* const*values, PQExpBuffer errorMessage, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 bool use_defaults, int expand_dbname); > =A0static char *conninfo_getval(PQconninfoOption *connOptions, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0const char= *keyword); > @@ -362,8 +362,8 @@ pgthreadlock_t pg_g_threadlock =3D default > =A0* call succeeded. > =A0*/ > =A0PGconn * > -PQconnectdbParams(const char **keywords, > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 const c= har **values, > +PQconnectdbParams(char const* const*keywords, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 char co= nst* const*values, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0int ex= pand_dbname) > =A0{ > =A0 =A0 =A0 =A0PGconn =A0 =A0 *conn =3D PQconnectStartParams(keywords, va= lues, expand_dbname); > @@ -381,8 +381,8 @@ PQconnectdbParams(const char **keywords, > =A0* check server status, accepting parameters identical to > PQconnectdbParams > =A0*/ > =A0PGPing > -PQpingParams(const char **keywords, > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0const char **values, > +PQpingParams(char const* const*keywords, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0char const* const*values, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 int expand_dbname) > =A0{ > =A0 =A0 =A0 =A0PGconn =A0 =A0 *conn =3D PQconnectStartParams(keywords, va= lues, expand_dbname); > @@ -464,8 +464,8 @@ PQping(const char *conninfo) > =A0* See PQconnectPoll for more info. > =A0*/ > =A0PGconn * > -PQconnectStartParams(const char **keywords, > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0const char **values, > +PQconnectStartParams(char const* const*keywords, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0char const* const*values, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 int expand_dbname) > =A0{ > =A0 =A0 =A0 =A0PGconn =A0 =A0 *conn; > @@ -4249,7 +4249,7 @@ conninfo_parse(const char *conninfo, PQE > =A0* keywords will take precedence, however. > =A0*/ > =A0static PQconninfoOption * > -conninfo_array_parse(const char **keywords, const char **values, > +conninfo_array_parse(char const* const*keywords, char const* const*value= s, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 PQExpBuffer errorMessage, bool use_defaults, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 int expand_dbname) > =A0{ > --- postgresql-9.1-9.1.0.orig/src/interfaces/libpq/libpq-fe.h > +++ postgresql-9.1-9.1.0/src/interfaces/libpq/libpq-fe.h > @@ -235,14 +235,14 @@ typedef struct pgresAttDesc > =A0/* make a new client connection to the backend */ > =A0/* Asynchronous (non-blocking) */ > =A0extern PGconn *PQconnectStart(const char *conninfo); > -extern PGconn *PQconnectStartParams(const char **keywords, > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0const char **values, int expand_dbname); > +extern PGconn *PQconnectStartParams(char const* const*keywords, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0char const* const*values, int expand_dbname); > =A0extern PostgresPollingStatusType PQconnectPoll(PGconn *conn); > > =A0/* Synchronous (blocking) */ > =A0extern PGconn *PQconnectdb(const char *conninfo); > -extern PGconn *PQconnectdbParams(const char **keywords, > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 const c= har **values, int expand_dbname); > +extern PGconn *PQconnectdbParams(char const* const*keywords, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 char co= nst* const*values, int expand_dbname); > =A0extern PGconn *PQsetdbLogin(const char *pghost, const char *pgport, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 const char *pgoptions, co= nst char *pgtty, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 const char *dbName, > @@ -413,8 +413,8 @@ extern int =A0PQsetnonblocking(PGconn *conn > =A0extern int =A0 =A0 PQisnonblocking(const PGconn *conn); > =A0extern int =A0 =A0 PQisthreadsafe(void); > =A0extern PGPing PQping(const char *conninfo); > -extern PGPing PQpingParams(const char **keywords, > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0const char **values, int= expand_dbname); > +extern PGPing PQpingParams(char const* const*keywords, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0char const* const*values= , int expand_dbname); > > =A0/* Force the write buffer to be written (or at least try) */ > =A0extern int =A0 =A0 PQflush(PGconn *conn); I agree with the rationale. Note query parameter arguments are already declared this way. merlin
On 21/09/2011 12:05 AM, Lionel Elie Mamane wrote: > The following bug has been logged online: > > Bug reference: 6216 > Logged by: Lionel Elie Mamane > Email address: lionel@mamane.lu > PostgreSQL version: 9.1.0 > Operating system: Debian GNU/Linux > Description: Calling PQconnectdbParams from C++ with a char** > Details: > > In C++, a "char**" value is not convertible to a "const char**" value, > because that is not safe (see > http://www.parashift.com/c++-faq-lite/const-correctness.html#faq-18.17). > > This means one cannot call libpq's PQconnectdbParams and friends passing > them a "char**" value for keywords and/or values, as these arguments are > declared "const char**". > > So please apply this patch, which declares them "char const* const*" > instead, which is: > - equivalent to "const char * const*"; use that one if you prefer Yeah, this really annoyed me when I was working with C libraries from C++. +1 from me; this should be applied. Lionel: Can I get you to add the patch to the commitfest app? https://commitfest.postgresql.org/ I'll accept review of it once you do and flag it as ready for committer so we can streamline its inclusion. As for wording: my *personal* preference is "const char * const" but I don't know what the opinions of those who work with the code day-to-day are. -- Craig Ringer
Craig Ringer <ringerc@ringerc.id.au> writes:
> As for wording: my *personal* preference is "const char * const" but I
> don't know what the opinions of those who work with the code day-to-day are.
+1. Isn't the other ordering deprecated by recent C standards?
(Or maybe I'm just thinking of where you're supposed to put "static",
but in any case "char const *" looks pretty weird to me.) Also,
the existing usages in libpq-fe.h look like that, and there's no good
reason for these to be randomly different.
regards, tom lane
ghtOn Wed, Sep 21, 2011 at 07:29:23AM +0800, Craig Ringer wrote: > On 21/09/2011 12:05 AM, Lionel Elie Mamane wrote: >>Bug reference: 6216 >>Logged by: Lionel Elie Mamane >>Email address: lionel@mamane.lu >> In C++, a "char**" value is not convertible to a "const char**" >> value, (...) >> This means one cannot call libpq's PQconnectdbParams and friends >> passing them a "char**" value for keywords and/or values, as these >> arguments are declared "const char**". > Lionel: Can I get you to add the patch to the commitfest app? > https://commitfest.postgresql.org/ Sure. Topic "performance" is not right, but only value I was allowed to put, and it wanted a value... I added my initial patch, and as far as I understand, I have to send the revised patch to the list before I can register it at the commitfest. So here is my revised patch, that uses "const char *const * " like elsewhere in the same file instead of "char const* const*". -- Lionel
Вложения
On 21/09/2011 4:25 PM, Lionel Elie Mamane wrote: > I added my initial patch, and as far as I understand, I have to send > the revised patch to the list before I can register it at the > commitfest. So here is my revised patch, that uses "const char *const * " > like elsewhere in the same file instead of "char const* const*". Yep, I'm happy with that. It does what it says and no more. I have *NOT* done any detailed testing, as I'm in the middle of moving house, but I find it hard to see how it can break anything that isn't its self incorrect. You can still pass a `char *' to a `const char * const' param, so it won't break any call sites. -- Craig Ringer
Craig Ringer <ringerc@ringerc.id.au> writes:
> On 21/09/2011 4:25 PM, Lionel Elie Mamane wrote:
>> I added my initial patch, and as far as I understand, I have to send
>> the revised patch to the list before I can register it at the
>> commitfest. So here is my revised patch, that uses "const char *const * "
>> like elsewhere in the same file instead of "char const* const*".
> Yep, I'm happy with that. It does what it says and no more.
I went ahead and committed this, since there seems no very good reason
to make it wait for the next commitfest.
regards, tom lane