Lists: | pgsql-bugs |
---|
From: | "Lionel Elie Mamane" <lionel(at)mamane(dot)lu> |
---|---|
To: | pgsql-bugs(at)postgresql(dot)org |
Subject: | BUG #6216: Calling PQconnectdbParams from C++ with a char** |
Date: | 2011-09-20 16:05:55 |
Message-ID: | 201109201605.p8KG5tku049175@wwwmaster.postgresql.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-bugs |
The following bug has been logged online:
Bug reference: 6216
Logged by: Lionel Elie Mamane
Email address: lionel(at)mamane(dot)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);
From: | Merlin Moncure <mmoncure(at)gmail(dot)com> |
---|---|
To: | Lionel Elie Mamane <lionel(at)mamane(dot)lu> |
Cc: | pgsql-bugs(at)postgresql(dot)org |
Subject: | Re: BUG #6216: Calling PQconnectdbParams from C++ with a char** |
Date: | 2011-09-20 23:29:01 |
Message-ID: | CAHyXU0zBa_2k+U1krDirt_rBp118oXt6pcB-SP+1wpfeZ2RZYA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-bugs |
On Tue, Sep 20, 2011 at 11:05 AM, Lionel Elie Mamane <lionel(at)mamane(dot)lu> wrote:
>
> The following bug has been logged online:
>
> Bug reference: 6216
> Logged by: Lionel Elie Mamane
> Email address: lionel(at)mamane(dot)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);
I agree with the rationale. Note query parameter arguments are
already declared this way.
merlin
From: | Craig Ringer <ringerc(at)ringerc(dot)id(dot)au> |
---|---|
To: | Lionel Elie Mamane <lionel(at)mamane(dot)lu> |
Cc: | pgsql-bugs(at)postgresql(dot)org |
Subject: | Re: BUG #6216: Calling PQconnectdbParams from C++ with a char** |
Date: | 2011-09-20 23:29:23 |
Message-ID: | 4E7921D3.7030209@ringerc.id.au |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-bugs |
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(at)mamane(dot)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
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Craig Ringer <ringerc(at)ringerc(dot)id(dot)au> |
Cc: | Lionel Elie Mamane <lionel(at)mamane(dot)lu>, pgsql-bugs(at)postgresql(dot)org |
Subject: | Re: BUG #6216: Calling PQconnectdbParams from C++ with a char** |
Date: | 2011-09-20 23:43:25 |
Message-ID: | 8393.1316562205@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-bugs |
Craig Ringer <ringerc(at)ringerc(dot)id(dot)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
From: | Lionel Elie Mamane <lionel(at)mamane(dot)lu> |
---|---|
To: | Craig Ringer <ringerc(at)ringerc(dot)id(dot)au> |
Cc: | pgsql-bugs(at)postgresql(dot)org |
Subject: | Re: BUG #6216: Calling PQconnectdbParams from C++ with a char** |
Date: | 2011-09-21 08:25:17 |
Message-ID: | 20110921082517.GA28080@capsaicin.mamane.lu |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-bugs |
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(at)mamane(dot)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
Attachment | Content-Type | Size |
---|---|---|
6216_constpp_v2.patch | text/x-diff | 3.6 KB |
From: | Craig Ringer <ringerc(at)ringerc(dot)id(dot)au> |
---|---|
To: | Lionel Elie Mamane <lionel(at)mamane(dot)lu> |
Cc: | pgsql-bugs(at)postgresql(dot)org |
Subject: | Re: BUG #6216: Calling PQconnectdbParams from C++ with a char** |
Date: | 2011-09-23 06:24:26 |
Message-ID: | 4E7C261A.3010908@ringerc.id.au |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-bugs |
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
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Craig Ringer <ringerc(at)ringerc(dot)id(dot)au> |
Cc: | Lionel Elie Mamane <lionel(at)mamane(dot)lu>, pgsql-bugs(at)postgresql(dot)org |
Subject: | Re: BUG #6216: Calling PQconnectdbParams from C++ with a char** |
Date: | 2011-09-25 22:54:39 |
Message-ID: | 9713.1316991279@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-bugs |
Craig Ringer <ringerc(at)ringerc(dot)id(dot)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