Change PQconndefaults() to return a malloc'd array, instead of a static

array.  This allows processing of conninfo strings to be made thread-safe,
at the cost of a small memory leak in applications that use
PQconndefaults() and are not updated to free the returned array via
the new PQconninfoFree() function.  But PQconndefaults() is probably not
used very much, so this seems like a good compromise.

src/interfaces/libpgtcl/pgtclCmds.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/interfaces/libpgtcl/Attic/pgtclCmds.c,v 1.47 2000/02/27 07:44:22 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/interfaces/libpgtcl/Attic/pgtclCmds.c,v 1.48 2000/03/11 03:08:35 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -237,25 +237,32 @@ tcl_value(char *value)
 int
 Pg_conndefaults(ClientData cData, Tcl_Interp *interp, int argc, char **argv)
 {
+	PQconninfoOption *options = PQconndefaults();
 	PQconninfoOption *option;
 	Tcl_DString result;
 	char		ibuf[32];
 
-	Tcl_DStringInit(&result);
-	for (option = PQconndefaults(); option->keyword != NULL; option++)
+	if (options)
 	{
-		char	   *val = option->val ? option->val : "";
+		Tcl_DStringInit(&result);
 
-		sprintf(ibuf, "%d", option->dispsize);
-		Tcl_DStringStartSublist(&result);
-		Tcl_DStringAppendElement(&result, option->keyword);
-		Tcl_DStringAppendElement(&result, option->label);
-		Tcl_DStringAppendElement(&result, option->dispchar);
-		Tcl_DStringAppendElement(&result, ibuf);
-		Tcl_DStringAppendElement(&result, val);
-		Tcl_DStringEndSublist(&result);
+		for (option = options; option->keyword != NULL; option++)
+		{
+			char	   *val = option->val ? option->val : "";
+
+			sprintf(ibuf, "%d", option->dispsize);
+			Tcl_DStringStartSublist(&result);
+			Tcl_DStringAppendElement(&result, option->keyword);
+			Tcl_DStringAppendElement(&result, option->label);
+			Tcl_DStringAppendElement(&result, option->dispchar);
+			Tcl_DStringAppendElement(&result, ibuf);
+			Tcl_DStringAppendElement(&result, val);
+			Tcl_DStringEndSublist(&result);
+		}
+		Tcl_DStringResult(interp, &result);
+
+		PQconninfoFree(options);
 	}
-	Tcl_DStringResult(interp, &result);
 
 	return TCL_OK;
 }

src/interfaces/libpq/fe-auth.c

@@ -10,7 +10,7 @@
  * exceed INITIAL_EXPBUFFER_SIZE (currently 256 bytes).
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/interfaces/libpq/fe-auth.c,v 1.37 2000/02/07 23:10:08 petere Exp $
+ *	  $Header: /cvsroot/pgsql/src/interfaces/libpq/fe-auth.c,v 1.38 2000/03/11 03:08:36 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -75,7 +75,7 @@ struct authsvc
  * allowed.  Unauthenticated connections are disallowed unless there
  * isn't any authentication system.
  */
-static struct authsvc authsvcs[] = {
+static const struct authsvc authsvcs[] = {
 #ifdef KRB4
 	{"krb4", STARTUP_KRB4_MSG, 1},
 	{"kerberos", STARTUP_KRB4_MSG, 1},
@@ -94,7 +94,7 @@ static struct authsvc authsvcs[] = {
 	{"password", STARTUP_PASSWORD_MSG, 0}
 };
 
-static int	n_authsvcs = sizeof(authsvcs) / sizeof(struct authsvc);
+static const int	n_authsvcs = sizeof(authsvcs) / sizeof(struct authsvc);
 
 #ifdef KRB4
 /*----------------------------------------------------------------
@@ -549,7 +549,14 @@ fe_sendauth(AuthRequest areq, PGconn *conn, const char *hostname,
  *
  * Set/return the authentication service currently selected for use by the
  * frontend. (You can only use one in the frontend, obviously.)
+ *
+ * NB: This is not thread-safe if different threads try to select different
+ * authentication services!  It's OK for fe_getauthsvc to select the default,
+ * since that will be the same for all threads, but direct application use
+ * of fe_setauthsvc is not thread-safe.  However, use of fe_setauthsvc is
+ * deprecated anyway...
  */
+
 static int	pg_authsvc = -1;
 
 void
@@ -558,7 +565,7 @@ fe_setauthsvc(const char *name, char *PQerrormsg)
 	int			i;
 
 	for (i = 0; i < n_authsvcs; ++i)
-		if (!strcmp(name, authsvcs[i].name))
+		if (strcmp(name, authsvcs[i].name) == 0)
 		{
 			pg_authsvc = i;
 			break;

src/interfaces/libpq/fe-exec.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v 1.91 2000/02/24 04:50:51 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v 1.92 2000/03/11 03:08:36 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -39,6 +39,7 @@ char * const pgresStatus[] = {
 };
 
 
+/* Note: DONOTICE macro will work if applied to either PGconn or PGresult */
 #define DONOTICE(conn,message) \
 	((*(conn)->noticeHook) ((conn)->noticeArg, (message)))
 
@@ -135,7 +136,7 @@ PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status)
 
 	result = (PGresult *) malloc(sizeof(PGresult));
 
-	result->conn = conn;		/* might be NULL */
+	result->xconn = conn;		/* might be NULL */
 	result->ntups = 0;
 	result->numAttributes = 0;
 	result->attDescs = NULL;
@@ -150,8 +151,11 @@ PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status)
 	result->curOffset = 0;
 	result->spaceLeft = 0;
 
-	if (conn)					/* consider copying conn's errorMessage */
+	if (conn)
 	{
+		result->noticeHook = conn->noticeHook;
+		result->noticeArg = conn->noticeArg;
+		/* consider copying conn's errorMessage */
 		switch (status)
 		{
 			case PGRES_EMPTY_QUERY:
@@ -166,6 +170,12 @@ PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status)
 				break;
 		}
 	}
+	else
+	{
+		result->noticeHook = NULL;
+		result->noticeArg = NULL;
+	}
+
 	return result;
 }
 
@@ -1833,12 +1843,12 @@ check_field_number(const char *routineName, const PGresult *res, int field_num)
 		return FALSE;			/* no way to display error message... */
 	if (field_num < 0 || field_num >= res->numAttributes)
 	{
-		if (res->conn)
+		if (res->noticeHook)
 		{
 			sprintf(noticeBuf,
 					"%s: ERROR! field number %d is out of range 0..%d\n",
 					routineName, field_num, res->numAttributes - 1);
-			DONOTICE(res->conn, noticeBuf);
+			DONOTICE(res, noticeBuf);
 		}
 		return FALSE;
 	}
@@ -1855,23 +1865,23 @@ check_tuple_field_number(const char *routineName, const PGresult *res,
 		return FALSE;			/* no way to display error message... */
 	if (tup_num < 0 || tup_num >= res->ntups)
 	{
-		if (res->conn)
+		if (res->noticeHook)
 		{
 			sprintf(noticeBuf,
 					"%s: ERROR! tuple number %d is out of range 0..%d\n",
 					routineName, tup_num, res->ntups - 1);
-			DONOTICE(res->conn, noticeBuf);
+			DONOTICE(res, noticeBuf);
 		}
 		return FALSE;
 	}
 	if (field_num < 0 || field_num >= res->numAttributes)
 	{
-		if (res->conn)
+		if (res->noticeHook)
 		{
 			sprintf(noticeBuf,
 					"%s: ERROR! field number %d is out of range 0..%d\n",
 					routineName, field_num, res->numAttributes - 1);
-			DONOTICE(res->conn, noticeBuf);
+			DONOTICE(res, noticeBuf);
 		}
 		return FALSE;
 	}
@@ -1982,11 +1992,11 @@ PQcmdStatus(PGresult *res)
 char *
 PQoidStatus(const PGresult *res)
 {
-        /* 
-         * This must be enough to hold the result. Don't laugh, this is
-         * better than what this function used to do.
-         */
-        static char buf[24];
+	/* 
+	 * This must be enough to hold the result. Don't laugh, this is
+	 * better than what this function used to do.
+	 */
+	static char buf[24];
 
 	size_t len;
 
@@ -1995,7 +2005,7 @@ PQoidStatus(const PGresult *res)
 
 	len = strspn(res->cmdStatus + 7, "0123456789");
 	if (len > 23)
-	        len = 23;
+		len = 23;
 	strncpy(buf, res->cmdStatus + 7, len);
 	buf[23] = '\0';
 
@@ -2046,12 +2056,12 @@ PQcmdTuples(PGresult *res)
 
 		if (*p == 0)
 		{
-			if (res->conn)
+			if (res->noticeHook)
 			{
 				sprintf(noticeBuf,
 						"PQcmdTuples (%s) -- bad input from server\n",
 						res->cmdStatus);
-				DONOTICE(res->conn, noticeBuf);
+				DONOTICE(res, noticeBuf);
 			}
 			return "";
 		}
@@ -2062,11 +2072,11 @@ PQcmdTuples(PGresult *res)
 			p++;				/* INSERT: skip oid */
 		if (*p == 0)
 		{
-			if (res->conn)
+			if (res->noticeHook)
 			{
 				sprintf(noticeBuf,
 					 "PQcmdTuples (INSERT) -- there's no # of tuples\n");
-				DONOTICE(res->conn, noticeBuf);
+				DONOTICE(res, noticeBuf);
 			}
 			return "";
 		}

src/interfaces/libpq/libpq-fe.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: libpq-fe.h,v 1.60 2000/02/07 23:10:11 petere Exp $
+ * $Id: libpq-fe.h,v 1.61 2000/03/11 03:08:37 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -130,6 +130,10 @@ extern		"C"
 
 /* ----------------
  * Structure for the conninfo parameter definitions returned by PQconndefaults
+ *
+ * All fields except "val" point at static strings which must not be altered.
+ * "val" is either NULL or a malloc'd current-value string.  PQconninfoFree()
+ * will release both the val strings and the PQconninfoOption array itself.
  * ----------------
  */
 	typedef struct _PQconninfoOption
@@ -137,14 +141,14 @@ extern		"C"
 		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	   *val;		/* Option's current value, or NULL		*/
 		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  */
-		/* "D"	Debug options - don't 	 */
-		/* create a field by default	*/
+		char	   *dispchar;	/* Character to display for this field
+								 * in a connect dialog. Values are:
+								 * ""	Display entered value as is
+								 * "*"	Password field - hide value
+								 * "D"	Debug option - don't show by default
+								 */
 		int			dispsize;	/* Field size in characters for dialog	*/
 	} PQconninfoOption;
 
@@ -183,11 +187,14 @@ extern		"C"
 #define PQsetdb(M_PGHOST,M_PGPORT,M_PGOPT,M_PGTTY,M_DBNAME)  \
 	PQsetdbLogin(M_PGHOST, M_PGPORT, M_PGOPT, M_PGTTY, M_DBNAME, NULL, NULL)
 
+	/* close the current connection and free the PGconn data structure */
+	extern void PQfinish(PGconn *conn);
+
 	/* get info about connection options known to PQconnectdb */
 	extern PQconninfoOption *PQconndefaults(void);
 
-	/* close the current connection and free the PGconn data structure */
-	extern void PQfinish(PGconn *conn);
+	/* free the data structure returned by PQconndefaults() */
+	extern void PQconninfoFree(PQconninfoOption *connOptions);
 
 	/*
 	 * close the current connection and restablish a new one with the same

src/interfaces/libpq/libpq-int.h

@@ -12,7 +12,7 @@
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: libpq-int.h,v 1.19 2000/02/07 23:10:11 petere Exp $
+ * $Id: libpq-int.h,v 1.20 2000/03/11 03:08:37 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -121,7 +121,21 @@ struct pg_result
 												 * last query */
 	int			binary;			/* binary tuple values if binary == 1,
 								 * otherwise ASCII */
-	PGconn	   *conn;			/* connection we did the query on, if any */
+	/*
+	 * The conn link in PGresult is no longer used by any libpq code.
+	 * It should be removed entirely, because it could be a dangling link
+	 * (the application could keep the PGresult around longer than it keeps
+	 * the PGconn!)  But there may be apps out there that depend on it,
+	 * so we will leave it here at least for a release or so.
+	 */
+	PGconn	   *xconn;			/* connection we did the query on, if any */
+
+	/* Callback procedure for notice/error message processing
+	 * (copied from originating PGconn).
+	 */
+	PQnoticeProcessor noticeHook;
+	void	   *noticeArg;
+
 	char	   *errMsg;			/* error message, or NULL if no error */
 
 	/* All NULL attributes in the query result point to this null string */

src/interfaces/libpq/libpqdll.def

@@ -78,3 +78,4 @@ EXPORTS
 	appendPQExpBufferStr	@ 75
 	destroyPQExpBuffer	@ 76
 	createPQExpBuffer	@ 77
+	PQconninfoFree		@ 78

src/interfaces/perl5/Pg.xs

 /*-------------------------------------------------------
  *
- * $Id: Pg.xs,v 1.13 1999/10/13 02:26:37 momjian Exp $ with patch for NULs
+ * $Id: Pg.xs,v 1.14 2000/03/11 03:08:37 tgl Exp $ with patch for NULs
  *
  * Copyright (c) 1997, 1998  Edmund Mergl
  *
@@ -247,17 +247,18 @@ PQsetdb(pghost, pgport, pgoptions, pgtty, dbname)
 HV *
 PQconndefaults()
 	CODE:
-		PQconninfoOption *infoOption;
+		PQconninfoOption *infoOptions;
 		RETVAL = newHV();
-                if (infoOption = PQconndefaults()) {
-			while (infoOption->keyword != NULL) {
-				if (infoOption->val != NULL) {
-					hv_store(RETVAL, infoOption->keyword, strlen(infoOption->keyword), newSVpv(infoOption->val, 0), 0);
+		if (infoOptions = PQconndefaults()) {
+			PQconninfoOption *option;
+			for (option = infoOptions; option->keyword != NULL; option++) {
+				if (option->val != NULL) {
+					hv_store(RETVAL, option->keyword, strlen(option->keyword), newSVpv(option->val, 0), 0);
 				} else {
-					hv_store(RETVAL, infoOption->keyword, strlen(infoOption->keyword), newSVpv("", 0), 0);
+					hv_store(RETVAL, option->keyword, strlen(option->keyword), newSVpv("", 0), 0);
 				}
-				infoOption++;
 			}
+			PQconninfoFree(infoOptions);
 		}
 	OUTPUT:
 		RETVAL
@@ -774,17 +775,18 @@ setdb(pghost, pgport, pgoptions, pgtty, dbname)
 HV *
 conndefaults()
 	CODE:
-		PQconninfoOption *infoOption;
+		PQconninfoOption *infoOptions;
 		RETVAL = newHV();
-                if (infoOption = PQconndefaults()) {
-			while (infoOption->keyword != NULL) {
-				if (infoOption->val != NULL) {
-					hv_store(RETVAL, infoOption->keyword, strlen(infoOption->keyword), newSVpv(infoOption->val, 0), 0);
+		if (infoOptions = PQconndefaults()) {
+			PQconninfoOption *option;
+			for (option = infoOptions; option->keyword != NULL; option++) {
+				if (option->val != NULL) {
+					hv_store(RETVAL, option->keyword, strlen(option->keyword), newSVpv(option->val, 0), 0);
 				} else {
-					hv_store(RETVAL, infoOption->keyword, strlen(infoOption->keyword), newSVpv("", 0), 0);
+					hv_store(RETVAL, option->keyword, strlen(option->keyword), newSVpv("", 0), 0);
 				}
-				infoOption++;
 			}
+			PQconninfoFree(infoOptions);
 		}
 	OUTPUT:
 		RETVAL