Re: [BUGS] BUG #6216: Calling PQconnectdbParams from C++ with a char**

2011-09-25 Thread Tom Lane
Craig Ringer  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

-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs


Re: [BUGS] BUG #6216: Calling PQconnectdbParams from C++ with a char**

2011-09-22 Thread Craig Ringer

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

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs


Re: [BUGS] BUG #6216: Calling PQconnectdbParams from C++ with a char**

2011-09-21 Thread Lionel Elie Mamane
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:  lio...@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
--- 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(const char *const * keywords,
+	 const char *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(const char *const * keywords,
+  const char *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(const char *const * keywords,
+			 const char *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(const char *const * keywords,
+	 const char *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(const char *const * keywords, const char *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(const char *const * keywords,
+	 const char *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(const char *const * keywords,
+  const char *const * , 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(const char *const * keywords,
+			 const char *const * values, int expand_dbname);
 
 /* Force the write buffer to be written (or at least try) */
 extern int	PQflush(PGconn *conn);

-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs


Re: [BUGS] BUG #6216: Calling PQconnectdbParams from C++ with a char**

2011-09-20 Thread Tom Lane
Craig Ringer  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

-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs


Re: [BUGS] BUG #6216: Calling PQconnectdbParams from C++ with a char**

2011-09-20 Thread Craig Ringer

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:  lio...@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

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs


Re: [BUGS] BUG #6216: Calling PQconnectdbParams from C++ with a char**

2011-09-20 Thread Merlin Moncure
On Tue, Sep 20, 2011 at 11:05 AM, Lionel Elie Mamane  wrote:
>
> The following bug has been logged online:
>
> Bug reference:      6216
> Logged by:          Lionel Elie Mamane
> Email address:      lio...@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 *pghos

[BUGS] BUG #6216: Calling PQconnectdbParams from C++ with a char**

2011-09-20 Thread Lionel Elie Mamane

The following bug has been logged online:

Bug reference:  6216
Logged by:  Lionel Elie Mamane
Email address:  lio...@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 i