I am sending this patch to hackers because I think it needs some
discussion before being added.  I'm not 100% sure that there
isn't some internal issue with making these changes but so far
it seems to work for me.

In interfaces/libpq/libpq-fe.h there are some structures that include
char pointers.  Often one would expect the user to send const strings
to the functions using these pointers.  The following keeps external
programs from failing when full error checking is enabled.


*** ../src.original/./interfaces/libpq/libpq-fe.h	Sat Jan 16 07:33:49 1999
--- ./interfaces/libpq/libpq-fe.h	Fri Jan 22 07:14:21 1999
***************
*** 100,108 ****
  		pqbool		html3;		/* output html tables */
  		pqbool		expanded;	/* expand tables */
  		pqbool		pager;		/* use pager for output if needed */
! 		char	   *fieldSep;	/* field separator */
! 		char	   *tableOpt;	/* insert to HTML <table ...> */
! 		char	   *caption;	/* HTML <caption> */
  		char	  **fieldName;	/* null terminated array of repalcement
  								 * field names */
  	} PQprintOpt;
--- 100,108 ----
  		pqbool		html3;		/* output html tables */
  		pqbool		expanded;	/* expand tables */
  		pqbool		pager;		/* use pager for output if needed */
! 		const char *fieldSep;	/* field separator */
! 		const char *tableOpt;	/* insert to HTML <table ...> */
! 		const char *caption;	/* HTML <caption> */
  		char	  **fieldName;	/* null terminated array of repalcement
  								 * field names */
  	} PQprintOpt;
***************
*** 113,124 ****
   */
  	typedef struct _PQconninfoOption
  	{
! 		char	   *keyword;	/* The keyword of the option			*/
! 		char	   *envvar;	/* Fallback environment variable name	*/
! 		char	   *compiled;	/* Fallback compiled in default value	*/
! 		char	   *val;		/* Options value						*/
! 		char	   *label;		/* Label for field in connect dialog	*/
! 		char	   *dispchar;	/* Character to display for this field	*/
  								/* in a connect dialog. Values are:		*/
  								/* ""	Display entered value as is  */
  								/* "*"	Password field - hide value  */
--- 113,124 ----
   */
  	typedef struct _PQconninfoOption
  	{
! 		const char	*keyword;	/* The keyword of the option			*/
! 		const char	*envvar;	/* Fallback environment variable name	*/
! 		const char	*compiled;	/* Fallback compiled in default value	*/
! 		char		*val;		/* Options value						*/
! 		const char	*label;		/* Label for field in connect dialog	*/
! 		const char	*dispchar;	/* Character to display for this field	*/
  								/* in a connect dialog. Values are:		*/
  								/* ""	Display entered value as is  */
  								/* "*"	Password field - hide value  */
*** ../src.original/./interfaces/libpq/fe-print.c	Fri Jan 22 07:02:10 1999
--- ./interfaces/libpq/fe-print.c	Fri Jan 22 07:03:09 1999
***************
*** 681,687 ****
  		p = border;
  		if (po->standard)
  		{
! 			char	   *fs = po->fieldSep;
  
  			while (*fs++)
  				*p++ = '+';
--- 681,687 ----
  		p = border;
  		if (po->standard)
  		{
! 			const char	   *fs = po->fieldSep;
  
  			while (*fs++)
  				*p++ = '+';
***************
*** 693,699 ****
  			for (len = fieldMax[j] + (po->standard ? 2 : 0); len--; *p++ = '-');
  			if (po->standard || (j + 1) < nFields)
  			{
! 				char	   *fs = po->fieldSep;
  
  				while (*fs++)
  					*p++ = '+';
--- 693,699 ----
  			for (len = fieldMax[j] + (po->standard ? 2 : 0); len--; *p++ = '-');
  			if (po->standard || (j + 1) < nFields)
  			{
! 				const char	   *fs = po->fieldSep;
  
  				while (*fs++)
  					*p++ = '+';
*** ../src.original/./interfaces/libpq/fe-connect.c	Fri Jan 22 07:04:03 1999
--- ./interfaces/libpq/fe-connect.c	Fri Jan 22 07:13:09 1999
***************
*** 48,54 ****
  static void freePGconn(PGconn *conn);
  static void closePGconn(PGconn *conn);
  static int	conninfo_parse(const char *conninfo, char *errorMessage);
! static char *conninfo_getval(char *keyword);
  static void conninfo_free(void);
  static void defaultNoticeProcessor(void *arg, const char *message);
  
--- 48,54 ----
  static void freePGconn(PGconn *conn);
  static void closePGconn(PGconn *conn);
  static int	conninfo_parse(const char *conninfo, char *errorMessage);
! static const char *conninfo_getval(const char *keyword);
  static void conninfo_free(void);
  static void defaultNoticeProcessor(void *arg, const char *message);
  
***************
*** 172,179 ****
  PGconn *
  PQconnectdb(const char *conninfo)
  {
! 	PGconn	   *conn;
! 	char	   *tmp;
  
  	/* ----------
  	 * Allocate memory for the conn structure
--- 172,179 ----
  PGconn *
  PQconnectdb(const char *conninfo)
  {
! 	PGconn		   *conn;
! 	const char	   *tmp;
  
  	/* ----------
  	 * Allocate memory for the conn structure
***************
*** 284,291 ****
  PGconn *
  PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions, const char *pgtty, const char *dbName, const char *login, const char *pwd)
  {
! 	PGconn	   *conn;
! 	char	   *tmp;
  
  	/* An error message from some service we call. */
  	bool		error = FALSE;
--- 284,291 ----
  PGconn *
  PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions, const char *pgtty, const char *dbName, const char *login, const char *pwd)
  {
! 	PGconn		*conn;
! 	const char	*tmp;
  
  	/* An error message from some service we call. */
  	bool		error = FALSE;
***************
*** 1137,1143 ****
  	char	   *pname;
  	char	   *pval;
  	char	   *buf;
! 	char	   *tmp;
  	char	   *cp;
  	char	   *cp2;
  	PQconninfoOption *option;
--- 1137,1143 ----
  	char	   *pname;
  	char	   *pval;
  	char	   *buf;
! 	const char *tmp;
  	char	   *cp;
  	char	   *cp2;
  	PQconninfoOption *option;
***************
*** 1343,1350 ****
  }
  
  
! static char *
! conninfo_getval(char *keyword)
  {
  	PQconninfoOption *option;
  
--- 1343,1350 ----
  }
  
  
! static const char *
! conninfo_getval(const char *keyword)
  {
  	PQconninfoOption *option;
  

-- 
D'Arcy J.M. Cain <darcy@{druid|vex}.net>   |  Democracy is three wolves
http://www.druid.net/darcy/                |  and a sheep voting on
+1 416 424 2871     (DoD#0082)    (eNTP)   |  what's for dinner.

Sat, 23 Jan 1999 07:42:19 -0500 (EST)
"D'Arcy" "J.M." Cain <darcy@druid.net>
Adding some const keywords to external interfaces
"D'Arcy" "J.M." Cain <darcy@druid.net> writes:
> In interfaces/libpq/libpq-fe.h there are some structures that include
> char pointers.  Often one would expect the user to send const strings
> to the functions using these pointers.  The following keeps external
> programs from failing when full error checking is enabled.

Yeah, I thought about const-ifying libpq's interfaces when I was working
on it last summer, but desisted for fear of breaking existing
application code.  The trouble with adding a few const keywords is that
they propagate.  Just as you had to change some of libpq's internal
variables from "char *" to "const char *" after modifying these structs,
so an application program would likely find that it needed to const-ify
some of its declarations to avoid errors/warnings created by this
change.  So, I didn't do it for fear of complaints.

IMHO, a partially const-ified program is worse than no consts at all;
you find yourself introducing casts all over the place to deal with
transitions between code that knows things are const and code that
doesn't use const.  So if we were going to do this I'd recommend doing
it whole-sale, and marking everything we could const in libpq-fe.h.
For example, most of the routines that accept or return char * really
ought to accept or return const char *; the pure inquiry functions ought
to take a const PGresult *, but not PQclear; etc.  But that would make
it even more likely that app programmers would be forced to clean up
code that is working fine for them now.  (We'd definitely have to clean
up the Postgres code that calls libpq.)

On the whole this seems like a can of worms better left unopened.
I certainly don't see it as something that one small patch will fix;
if we want to take const-safety seriously the effects will ripple
throughout the code...

			regards, tom lane

Sat, 23 Jan 1999 13:27:56 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] Adding some const keywords to external interfaces 
> Yeah, I thought about const-ifying libpq's interfaces when I was working
> on it last summer, but desisted for fear of breaking existing
> application code.  The trouble with adding a few const keywords is that
> they propagate.  Just as you had to change some of libpq's internal
> variables from "char *" to "const char *" after modifying these structs,
> so an application program would likely find that it needed to const-ify
> some of its declarations to avoid errors/warnings created by this
> change.  So, I didn't do it for fear of complaints.
> 
> IMHO, a partially const-ified program is worse than no consts at all;
> you find yourself introducing casts all over the place to deal with
> transitions between code that knows things are const and code that
> doesn't use const.  So if we were going to do this I'd recommend doing
> it whole-sale, and marking everything we could const in libpq-fe.h.
> For example, most of the routines that accept or return char * really
> ought to accept or return const char *; the pure inquiry functions ought
> to take a const PGresult *, but not PQclear; etc.  But that would make
> it even more likely that app programmers would be forced to clean up
> code that is working fine for them now.  (We'd definitely have to clean
> up the Postgres code that calls libpq.)
> 
> On the whole this seems like a can of worms better left unopened.
> I certainly don't see it as something that one small patch will fix;
> if we want to take const-safety seriously the effects will ripple
> throughout the code...

Well said.  I have always suspected const would ripple through the code,
but had never heard it described so well.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Sat, 23 Jan 1999 16:35:58 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Adding some const keywords to external interfaces
Thus spake Tom Lane
> "D'Arcy" "J.M." Cain <darcy@druid.net> writes:
> > In interfaces/libpq/libpq-fe.h there are some structures that include
> > char pointers.  Often one would expect the user to send const strings
> > to the functions using these pointers.  The following keeps external
> > programs from failing when full error checking is enabled.
> 
> Yeah, I thought about const-ifying libpq's interfaces when I was working
> on it last summer, but desisted for fear of breaking existing
> application code.  The trouble with adding a few const keywords is that
> they propagate.  Just as you had to change some of libpq's internal
> variables from "char *" to "const char *" after modifying these structs,
> so an application program would likely find that it needed to const-ify
> some of its declarations to avoid errors/warnings created by this
> change.  So, I didn't do it for fear of complaints.

Actually, all the changes should be internal to our own code.  Functions
that take const char pointers can still send non-const pointers.  The
errors would be generated  if we changed the return value of the library
functions, not the arguments.

Not that I don't think that return values should be changed where it
is appropriate as well but this change doesn't do that.

> code that is working fine for them now.  (We'd definitely have to clean
> up the Postgres code that calls libpq.)

Like what?  I compiled the whole tree including my PyGreSQL module
and all the changes were inside libpq as I expected.

> if we want to take const-safety seriously the effects will ripple
> throughout the code...

I would love to see const-safety given more attention but certainly
we should make sure our external interfaces are correct so that people
writing to them have the opportunity to do full error checking.

-- 
D'Arcy J.M. Cain <darcy@{druid|vex}.net>   |  Democracy is three wolves
http://www.druid.net/darcy/                |  and a sheep voting on
+1 416 424 2871     (DoD#0082)    (eNTP)   |  what's for dinner.

Sat, 23 Jan 1999 22:42:39 -0500 (EST)
"D'Arcy" "J.M." Cain <darcy@druid.net>
Re: [HACKERS] Adding some const keywords to external interfaces
> Actually, all the changes should be internal to our own code.  Functions
> that take const char pointers can still send non-const pointers.  The
> errors would be generated  if we changed the return value of the library
> functions, not the arguments.
> 
> Not that I don't think that return values should be changed where it
> is appropriate as well but this change doesn't do that.
> 
...
> > if we want to take const-safety seriously the effects will ripple
> > throughout the code...
> 
> I would love to see const-safety given more attention but certainly
> we should make sure our external interfaces are correct so that people
> writing to them have the opportunity to do full error checking.

These are good points.  Can you post the patch again?  I deleted it. 
Sounds like it would be safe.  I am interested in const-ify-ing the
backend code, if possible.  It does offer a level of code checking that
we don't currently have.

The only issue is that is has to be done pretty exhaustively.  If you
don't, your new const function parameters start passing params to
functions that takes non-const params, and warnings start to fly.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Sat, 23 Jan 1999 23:33:01 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Adding some const keywords to external interfaces
Thus spake Bruce Momjian
> These are good points.  Can you post the patch again?  I deleted it. 

I bounced it directly to you rather than reposting to the list.

> Sounds like it would be safe.  I am interested in const-ify-ing the
> backend code, if possible.  It does offer a level of code checking that
> we don't currently have.

Me too but as I said, this patch doesn't do that.  It only const-ifies
the the arguments to an external interface.

> The only issue is that is has to be done pretty exhaustively.  If you
> don't, your new const function parameters start passing params to
> functions that takes non-const params, and warnings start to fly.

I compiled the entire tree without any warnings so I assume that the
changes wound up being pretty localized.

-- 
D'Arcy J.M. Cain <darcy@{druid|vex}.net>   |  Democracy is three wolves
http://www.druid.net/darcy/                |  and a sheep voting on
+1 416 424 2871     (DoD#0082)    (eNTP)   |  what's for dinner.

Sun, 24 Jan 1999 09:22:18 -0500 (EST)
"D'Arcy" "J.M." Cain <darcy@druid.net>
Re: [HACKERS] Adding some const keywords to external interfaces
"D'Arcy" "J.M." Cain <darcy@druid.net> writes:
> Actually, all the changes should be internal to our own code.  Functions
> that take const char pointers can still send non-const pointers.  The
> errors would be generated  if we changed the return value of the library
> functions, not the arguments.

True, adding const decorations to function arguments is fairly harmless
from the caller's point of view, but it also provides only a small
fraction of the error checking available with a fully const-ified
interface.

More to the point, the patch you submitted was *not* adding consts to
function arguments, it was adding consts to struct fields.  That *can*
cause errors in calling code, if the caller happens to copy the value
of such a field into a local variable that's not declared const, pass
it as an argument to a function not marked const, etc.

I guess my question is "why start here?".

>> if we want to take const-safety seriously the effects will ripple
>> throughout the code...

> I would love to see const-safety given more attention but certainly
> we should make sure our external interfaces are correct so that people
> writing to them have the opportunity to do full error checking.

Well, that's exactly my point.  I don't see much value in doing a
half-baked job of decorating the interface with const declarations;
you don't get much real error checking that way.  If we are going to
take const-safety seriously then we need to do the whole job.

That's a fair amount of work that will impact outside applications as
well as a lot of our own code (certainly most of interfaces/ and bin/,
more if we start const-ifying the backend's internal interfaces).
I think we need a pgsql-hackers consensus and commitment to the idea
before we start doing it.

			regards, tom lane

Sun, 24 Jan 1999 13:19:18 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] Adding some const keywords to external interfaces 
We agreed to skip this right now, right?


> Thus spake Bruce Momjian
> > These are good points.  Can you post the patch again?  I deleted it. 
> 
> I bounced it directly to you rather than reposting to the list.
> 
> > Sounds like it would be safe.  I am interested in const-ify-ing the
> > backend code, if possible.  It does offer a level of code checking that
> > we don't currently have.
> 
> Me too but as I said, this patch doesn't do that.  It only const-ifies
> the the arguments to an external interface.
> 
> > The only issue is that is has to be done pretty exhaustively.  If you
> > don't, your new const function parameters start passing params to
> > functions that takes non-const params, and warnings start to fly.
> 
> I compiled the entire tree without any warnings so I assume that the
> changes wound up being pretty localized.
> 
> -- 
> D'Arcy J.M. Cain <darcy@{druid|vex}.net>   |  Democracy is three wolves
> http://www.druid.net/darcy/                |  and a sheep voting on
> +1 416 424 2871     (DoD#0082)    (eNTP)   |  what's for dinner.
> 
> 


-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Tue, 2 Feb 1999 13:36:11 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Adding some const keywords to external interfaces
Thus spake Bruce Momjian
> We agreed to skip this right now, right?

I still think it's benign at worst.  Shall I keepthe changes and resubmit
later?


> > Thus spake Bruce Momjian
> > > These are good points.  Can you post the patch again?  I deleted it. 
> > 
> > I bounced it directly to you rather than reposting to the list.
> > 
> > > Sounds like it would be safe.  I am interested in const-ify-ing the
> > > backend code, if possible.  It does offer a level of code checking that
> > > we don't currently have.
> > 
> > Me too but as I said, this patch doesn't do that.  It only const-ifies
> > the the arguments to an external interface.
> > 
> > > The only issue is that is has to be done pretty exhaustively.  If you
> > > don't, your new const function parameters start passing params to
> > > functions that takes non-const params, and warnings start to fly.
> > 
> > I compiled the entire tree without any warnings so I assume that the
> > changes wound up being pretty localized.


-- 
D'Arcy J.M. Cain <darcy@{druid|vex}.net>   |  Democracy is three wolves
http://www.druid.net/darcy/                |  and a sheep voting on
+1 416 424 2871     (DoD#0082)    (eNTP)   |  what's for dinner.

Tue, 2 Feb 1999 13:58:28 -0500 (EST)
"D'Arcy" "J.M." Cain <darcy@druid.net>
Re: [HACKERS] Adding some const keywords to external interfaces
> 
> > > Thus spake Bruce Momjian
> > > > These are good points.  Can you post the patch again?  I deleted it. 
> > > 
> > > I bounced it directly to you rather than reposting to the list.
> > > 
> > > > Sounds like it would be safe.  I am interested in const-ify-ing the
> > > > backend code, if possible.  It does offer a level of code checking that
> > > > we don't currently have.
> > > 
> > > Me too but as I said, this patch doesn't do that.  It only const-ifies
> > > the the arguments to an external interface.
> > > 
> > > > The only issue is that is has to be done pretty exhaustively.  If you
> > > > don't, your new const function parameters start passing params to
> > > > functions that takes non-const params, and warnings start to fly.
> > > 
> > > I compiled the entire tree without any warnings so I assume that the
> > > changes wound up being pretty localized.

> Thus spake Bruce Momjian
> > We agreed to skip this right now, right?
> 
> I still think it's benign at worst.  Shall I keepthe changes and resubmit
> later?
> 

Didn't we agree we have to do all the const stuff at once?  And people
addressing those internal fields would now find them to be const?  I
really don't remember.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Tue, 2 Feb 1999 14:19:13 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Adding some const keywords to external interfaces
Um, I hate to say "I told you so", but this const-addition has
in fact created more warning messages than it eliminated:

gcc -I../../interfaces/libpq -I../../include -I../../backend   -g -O -Wall -Wmissing-prototypes   -c psql.c -o psql.o
psql.c: In function `HandleSlashCmds':
psql.c:1833: warning: passing arg 1 of `free' discards `const' from pointer target type
psql.c:2192: warning: passing arg 1 of `free' discards `const' from pointer target type
psql.c:2257: warning: passing arg 1 of `free' discards `const' from pointer target type
psql.c:2265: warning: passing arg 1 of `free' discards `const' from pointer target type
psql.c:2309: warning: passing arg 1 of `free' discards `const' from pointer target type
psql.c: In function `main':
psql.c:2982: warning: passing arg 1 of `free' discards `const' from pointer target type

I think this is a fairly graphic demonstration of my assertion that
adding const to application-visible fields is not entirely transparent.

I would like to back this patch out until such time as we are prepared
to fully const-ify libpq's interface (eg, declare PQgetvalue and all
the other accessor functions as returning const char* not just char*).
We shouldn't annoy application programmers a little bit here and a
little bit there --- we should go the whole nine yards at once rather
than forcing them to insert a few more "const"s with each release.
IMHO, anyway.

			regards, tom lane

Thu, 04 Feb 1999 23:11:45 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] Adding some const keywords to external interfaces 
I see the problem here too.  Reversing it now out.

D'Arcy, do you care to comment?


> Um, I hate to say "I told you so", but this const-addition has
> in fact created more warning messages than it eliminated:
> 
> gcc -I../../interfaces/libpq -I../../include -I../../backend   -g -O -Wall -Wmissing-prototypes   -c psql.c -o psql.o
> psql.c: In function `HandleSlashCmds':
> psql.c:1833: warning: passing arg 1 of `free' discards `const' from pointer target type
> psql.c:2192: warning: passing arg 1 of `free' discards `const' from pointer target type
> psql.c:2257: warning: passing arg 1 of `free' discards `const' from pointer target type
> psql.c:2265: warning: passing arg 1 of `free' discards `const' from pointer target type
> psql.c:2309: warning: passing arg 1 of `free' discards `const' from pointer target type
> psql.c: In function `main':
> psql.c:2982: warning: passing arg 1 of `free' discards `const' from pointer target type
> 
> I think this is a fairly graphic demonstration of my assertion that
> adding const to application-visible fields is not entirely transparent.
> 
> I would like to back this patch out until such time as we are prepared
> to fully const-ify libpq's interface (eg, declare PQgetvalue and all
> the other accessor functions as returning const char* not just char*).
> We shouldn't annoy application programmers a little bit here and a
> little bit there --- we should go the whole nine yards at once rather
> than forcing them to insert a few more "const"s with each release.
> IMHO, anyway.
> 
> 			regards, tom lane
> 


-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Thu, 4 Feb 1999 23:26:36 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Adding some const keywords to external interfaces