libpq was not consistently checking for memory allocation failures. This

patch adds missing checks to the call sites of malloc(), strdup(),
PQmakeEmptyPGresult(), pqResultAlloc(), and pqResultStrdup(), and updates
the documentation. Per original report from Volkan Yazici about
PQmakeEmptyPGresult() not checking for malloc() failure.

Author: Neil Conway

doc/src/sgml/libpq.sgml

@@ -1,5 +1,5 @@
 <!--
-$PostgreSQL: pgsql/doc/src/sgml/libpq.sgml,v 1.184 2005/06/10 03:02:01 momjian Exp $
+$PostgreSQL: pgsql/doc/src/sgml/libpq.sgml,v 1.185 2005/06/12 00:00:20 neilc Exp $
 -->
 
  <chapter id="libpq">
@@ -581,14 +581,15 @@ typedef struct
 </para>
 
 <para>
-   Returns a connection options array.  This may
-   be used to determine all possible <function>PQconnectdb</function> options and their
+   Returns a connection options array.  This may be used to determine
+   all possible <function>PQconnectdb</function> options and their
    current default values.  The return value points to an array of
-   <structname>PQconninfoOption</structname> structures, which ends with an entry having a null
-   <structfield>keyword</> pointer.  Note that the current default values
-   (<structfield>val</structfield> fields)
-   will depend on environment variables and other context.
-   Callers must treat the connection options data as read-only.
+   <structname>PQconninfoOption</structname> structures, which ends
+   with an entry having a null <structfield>keyword</> pointer.  The
+   null pointer is returned if memory could not be allocated. Note that
+   the current default values (<structfield>val</structfield> fields)
+   will depend on environment variables and other context.  Callers
+   must treat the connection options data as read-only.
    </para>
 
    <para>
@@ -1651,18 +1652,22 @@ void PQclear(PGresult *res);
 <para>
           Constructs an empty <structname>PGresult</structname> object with the given status.
 <synopsis>
-PGresult* PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status);
+PGresult *PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status);
 </synopsis>
 </para>
 
 <para>
-This is <application>libpq</>'s internal function to allocate and initialize an empty
-<structname>PGresult</structname> object.  It is exported because some applications find it
-useful to generate result objects (particularly objects with error
-status) themselves.  If <parameter>conn</parameter> is not null and <parameter>status</> indicates an error,
-the current error message of the specified connection is copied into the <structname>PGresult</structname>.
-Note that <function>PQclear</function> should eventually be called on the object, just
-as with a <structname>PGresult</structname> returned by <application>libpq</application> itself.
+This is <application>libpq</>'s internal function to allocate and
+initialize an empty <structname>PGresult</structname> object.  This
+function returns NULL if memory could not be allocated. It is exported
+because some applications find it useful to generate result objects
+(particularly objects with error status) themselves.  If
+<parameter>conn</parameter> is not null and <parameter>status</>
+indicates an error, the current error message of the specified
+connection is copied into the <structname>PGresult</structname>.  Note
+that <function>PQclear</function> should eventually be called on the
+object, just as with a <structname>PGresult</structname> returned by
+<application>libpq</application> itself.
 </para>
 </listitem>
 </varlistentry>
@@ -2266,15 +2271,15 @@ unsigned char *PQescapeBytea(const unsigned char *from,
   <para>
    <function>PQescapeBytea</> returns an escaped version of the
    <parameter>from</parameter> parameter binary string in memory
-   allocated with <function>malloc()</>.  This memory must be freed
-   using <function>PQfreemem</> when the result is no longer needed.
-   The return string has all special characters replaced so that they
-   can be properly processed by the
-   <productname>PostgreSQL</productname> string literal parser, and
-   the <type>bytea</type> input function. A terminating zero byte is
-   also added.  The single quotes that must surround
-   <productname>PostgreSQL</productname> string literals are not part
-   of the result string.
+   allocated with <function>malloc()</> (a null pointer is returned if
+   memory could not be allocated).  This memory must be freed using
+   <function>PQfreemem</> when the result is no longer needed.  The
+   return string has all special characters replaced so that they can
+   be properly processed by the <productname>PostgreSQL</productname>
+   string literal parser, and the <type>bytea</type> input function. A
+   terminating zero byte is also added.  The single quotes that must
+   surround <productname>PostgreSQL</productname> string literals are
+   not part of the result string.
   </para>
   </listitem>
   </varlistentry>

src/interfaces/libpq/fe-connect.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-connect.c,v 1.309 2005/06/10 04:01:36 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-connect.c,v 1.310 2005/06/12 00:00:21 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -368,6 +368,8 @@ connectOptions1(PGconn *conn, const char *conninfo)
 	 *
 	 * Don't put anything cute here --- intelligence should be in
 	 * connectOptions2 ...
+	 *
+	 * XXX: probably worth checking strdup() return value here...
 	 */
 	tmp = conninfo_getval(connOptions, "hostaddr");
 	conn->pghostaddr = tmp ? strdup(tmp) : NULL;
@@ -459,7 +461,6 @@ connectOptions2(PGconn *conn)
 	}
 
 #ifdef NOT_USED
-
 	/*
 	 * parse dbName to get all additional info in it, if any
 	 */
@@ -2167,11 +2168,9 @@ closePGconn(PGconn *conn)
 }
 
 /*
-   PQfinish:
-	  properly close a connection to the backend
-	  also frees the PGconn data structure so it shouldn't be re-used
-	  after this
-*/
+ * PQfinish: properly close a connection to the backend. Also frees
+ * the PGconn data structure so it shouldn't be re-used after this.
+ */
 void
 PQfinish(PGconn *conn)
 {
@@ -2182,10 +2181,10 @@ PQfinish(PGconn *conn)
 	}
 }
 
-/* PQreset :
-   resets the connection to the backend
-   closes the existing connection and makes a new one
-*/
+/*
+ * PQreset: resets the connection to the backend by closing the
+ * existing connection and creating a new one.
+ */
 void
 PQreset(PGconn *conn)
 {
@@ -2199,11 +2198,12 @@ PQreset(PGconn *conn)
 }
 
 
-/* PQresetStart :
-   resets the connection to the backend
-   closes the existing connection and makes a new one
-   Returns 1 on success, 0 on failure.
-*/
+/*
+ * PQresetStart:
+ * resets the connection to the backend
+ * closes the existing connection and makes a new one
+ * Returns 1 on success, 0 on failure.
+ */
 int
 PQresetStart(PGconn *conn)
 {
@@ -2218,11 +2218,11 @@ PQresetStart(PGconn *conn)
 }
 
 
-/* PQresetPoll :
-   resets the connection to the backend
-   closes the existing connection and makes a new one
-*/
-
+/*
+ * PQresetPoll:
+ * resets the connection to the backend
+ * closes the existing connection and makes a new one
+ */
 PostgresPollingStatusType
 PQresetPoll(PGconn *conn)
 {
@@ -2514,7 +2514,7 @@ parseServiceInfo(PQconninfoOption *options, PQExpBuffer errorMessage)
 	 * location to find our config files.
 	 */
 	snprintf(serviceFile, MAXPGPATH, "%s/pg_service.conf",
-		   getenv("PGSYSCONFDIR") ? getenv("PGSYSCONFDIR") : SYSCONFDIR);
+			 getenv("PGSYSCONFDIR") ? getenv("PGSYSCONFDIR") : SYSCONFDIR);
 
 	if (service != NULL)
 	{
@@ -2802,7 +2802,14 @@ conninfo_parse(const char *conninfo, PQExpBuffer errorMessage)
 		if (option->val)
 			free(option->val);
 		option->val = strdup(pval);
-
+		if (!option->val)
+		{
+			printfPQExpBuffer(errorMessage,
+							  libpq_gettext("out of memory\n"));
+			PQconninfoFree(options);
+			free(buf);
+			return NULL;
+		}
 	}
 
 	/* Done with the modifiable input string */
@@ -2835,6 +2842,13 @@ conninfo_parse(const char *conninfo, PQExpBuffer errorMessage)
 			if ((tmp = getenv(option->envvar)) != NULL)
 			{
 				option->val = strdup(tmp);
+				if (!option->val)
+				{
+					printfPQExpBuffer(errorMessage,
+									  libpq_gettext("out of memory\n"));
+					PQconninfoFree(options);
+					return NULL;
+				}
 				continue;
 			}
 		}
@@ -2846,6 +2860,13 @@ conninfo_parse(const char *conninfo, PQExpBuffer errorMessage)
 		if (option->compiled != NULL)
 		{
 			option->val = strdup(option->compiled);
+			if (!option->val)
+			{
+				printfPQExpBuffer(errorMessage,
+								  libpq_gettext("out of memory\n"));
+				PQconninfoFree(options);
+				return NULL;
+			}
 			continue;
 		}
 

src/interfaces/libpq/fe-exec.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-exec.c,v 1.168 2005/06/09 20:01:16 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-exec.c,v 1.169 2005/06/12 00:00:21 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -134,6 +134,8 @@ PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status)
 	PGresult   *result;
 
 	result = (PGresult *) malloc(sizeof(PGresult));
+	if (!result)
+		return NULL;
 
 	result->ntups = 0;
 	result->numAttributes = 0;
@@ -453,7 +455,7 @@ pqPrepareAsyncResult(PGconn *conn)
  * a trailing newline, and should not be more than one line).
  */
 void
-pqInternalNotice(const PGNoticeHooks *hooks, const char *fmt,...)
+pqInternalNotice(const PGNoticeHooks *hooks, const char *fmt, ...)
 {
 	char		msgBuf[1024];
 	va_list		args;
@@ -470,6 +472,8 @@ pqInternalNotice(const PGNoticeHooks *hooks, const char *fmt,...)
 
 	/* Make a PGresult to pass to the notice receiver */
 	res = PQmakeEmptyPGresult(NULL, PGRES_NONFATAL_ERROR);
+	if (!res)
+		return;
 	res->noticeHooks = *hooks;
 
 	/*
@@ -480,15 +484,19 @@ pqInternalNotice(const PGNoticeHooks *hooks, const char *fmt,...)
 	/* XXX should provide a SQLSTATE too? */
 
 	/*
-	 * Result text is always just the primary message + newline.
+	 * Result text is always just the primary message + newline. If we
+	 * can't allocate it, don't bother invoking the receiver.
 	 */
 	res->errMsg = (char *) pqResultAlloc(res, strlen(msgBuf) + 2, FALSE);
-	sprintf(res->errMsg, "%s\n", msgBuf);
+	if (res->errMsg)
+	{
+		sprintf(res->errMsg, "%s\n", msgBuf);
 
-	/*
-	 * Pass to receiver, then free it.
-	 */
-	(*res->noticeHooks.noticeRec) (res->noticeHooks.noticeRecArg, res);
+		/*
+		 * Pass to receiver, then free it.
+		 */
+		(*res->noticeHooks.noticeRec) (res->noticeHooks.noticeRecArg, res);
+	}
 	PQclear(res);
 }
 
@@ -1127,8 +1135,9 @@ PQisBusy(PGconn *conn)
 
 /*
  * PQgetResult
- *	  Get the next PGresult produced by a query.
- *	  Returns NULL if and only if no query work remains.
+ *	  Get the next PGresult produced by a query.  Returns NULL if no
+ *	  query work remains or an error has occurred (e.g. out of
+ *	  memory).
  */
 
 PGresult *

src/interfaces/libpq/fe-misc.c

@@ -23,7 +23,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-misc.c,v 1.113 2005/02/22 04:42:20 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-misc.c,v 1.114 2005/06/12 00:00:21 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -175,7 +175,8 @@ pqGetnchar(char *s, size_t len, PGconn *conn)
 	conn->inCursor += len;
 
 	if (conn->Pfdebug)
-		fprintf(conn->Pfdebug, libpq_gettext("From backend (%lu)> %.*s\n"), (unsigned long) len, (int) len, s);
+		fprintf(conn->Pfdebug, libpq_gettext("From backend (%lu)> %.*s\n"),
+				(unsigned long) len, (int) len, s);
 
 	return 0;
 }

src/interfaces/libpq/fe-print.c

@@ -10,7 +10,7 @@
  * didn't really belong there.
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-print.c,v 1.59 2005/02/22 04:42:20 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-print.c,v 1.60 2005/06/12 00:00:21 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -62,14 +62,11 @@ static void fill(int length, int max, char filler, FILE *fp);
  * details
  *
  * This function should probably be removed sometime since psql
- * doesn't use it anymore. It is unclear to what extend this is used
+ * doesn't use it anymore. It is unclear to what extent this is used
  * by external clients, however.
  */
-
 void
-PQprint(FILE *fout,
-		const PGresult *res,
-		const PQprintOpt *po)
+PQprint(FILE *fout, const PGresult *res, const PQprintOpt *po)
 {
 	int			nFields;
 
@@ -611,6 +608,12 @@ PQdisplayTuples(const PGresult *res,
 	if (fillAlign)
 	{
 		fLength = (int *) malloc(nFields * sizeof(int));
+		if (!fLength)
+		{
+			fprintf(stderr, libpq_gettext("out of memory\n"));
+			exit(1);
+		}
+
 		for (j = 0; j < nFields; j++)
 		{
 			fLength[j] = strlen(PQfname(res, j));
@@ -705,6 +708,11 @@ PQprintTuples(const PGresult *res,
 
 			width = nFields * 14;
 			tborder = malloc(width + 1);
+			if (!tborder)
+			{
+				fprintf(stderr, libpq_gettext("out of memory\n"));
+				exit(1);
+			}
 			for (i = 0; i <= width; i++)
 				tborder[i] = '-';
 			tborder[i] = '\0';