GCC 4.0 includes a new warning option, -Wformat-literal, that emits

Neil Conway · Neil Conway · commit a4c3f7dd192f · 2005-04-30T09:08:14.000Z
a warning when a variable is used as a format string for printf()
and similar functions (if the variable is derived from untrusted
data, it could include unexpected formatting sequences). This
emits too many warnings to be enabled by default, but it does
flag a few dubious constructs in the Postgres tree. This patch
fixes up the obvious variants: functions that are passed a variable
format string but no additional arguments.

This patch fixes a bug in pg_dump (triggers with formatting sequences
in their names are not dumped correctly) and some related pg_dump
code that looks dubious; cleanups for more harmless instances have
been applied to more recent branches. This patch also fixes an
additional format string bug that is present in 7.2 but not in later
releases: pg_dump would also fail to correctly dump indexes with
formatting sequences in their names.
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
@@ -15,7 +15,7 @@
  *
  *
  * IDENTIFICATION
- *		$Header: /cvsroot/pgsql/src/bin/pg_dump/pg_backup_archiver.c,v 1.42 2002/02/11 00:18:20 tgl Exp $
+ *		$Header: /cvsroot/pgsql/src/bin/pg_dump/pg_backup_archiver.c,v 1.42.2.1 2005/04/30 09:08:14 neilc Exp $
  *
  * Modifications - 28-Jun-2000 - pjw@rhyme.com.au
  *
@@ -391,7 +391,7 @@ RestoreArchive(Archive *AHX, RestoreOptions *ropt)
 						 * mode with libpq.
 						 */
 						if (te->copyStmt && strlen(te->copyStmt) > 0)
-							ahprintf(AH, te->copyStmt);
+							ahprintf(AH, "%s", te->copyStmt);
 
 						(*AH->PrintTocDataPtr) (AH, te, ropt);
 
@@ -2006,7 +2006,7 @@ _reconnectAsUser(ArchiveHandle *AH, const char *dbname, const char *user)
 		appendPQExpBuffer(qry, " %s\n\n",
 						  fmtId(user, false));
 
-		ahprintf(AH, qry->data);
+		ahprintf(AH, "%s", qry->data);
 
 		destroyPQExpBuffer(qry);
 	}
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
@@ -22,7 +22,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/bin/pg_dump/pg_dump.c,v 1.241.2.3 2004/03/20 18:12:32 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/bin/pg_dump/pg_dump.c,v 1.241.2.4 2005/04/30 09:08:14 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -436,7 +436,7 @@ dumpClasses_dumpData(Archive *fout, char *oid, void *dctxv)
 				{
 					if (field > 0)
 						appendPQExpBuffer(q, ",");
-					appendPQExpBuffer(q, fmtId(PQfname(res, field), force_quotes));
+					appendPQExpBufferStr(q, fmtId(PQfname(res, field), force_quotes));
 				}
 				appendPQExpBuffer(q, ") ");
 				archprintf(fout, "%s", q->data);
@@ -2599,12 +2599,12 @@ getTables(int *numTables, FuncInfo *finfo, int numFuncs, const char *tablename)
 				if (tgisconstraint)
 				{
 					appendPQExpBuffer(query, "CREATE CONSTRAINT TRIGGER ");
-					appendPQExpBuffer(query, fmtId(PQgetvalue(res2, i2, i_tgconstrname), force_quotes));
+					appendPQExpBufferStr(query, fmtId(PQgetvalue(res2, i2, i_tgconstrname), force_quotes));
 				}
 				else
 				{
 					appendPQExpBuffer(query, "CREATE TRIGGER ");
-					appendPQExpBuffer(query, fmtId(tgname, force_quotes));
+					appendPQExpBufferStr(query, fmtId(tgname, force_quotes));
 				}
 				appendPQExpBufferChar(query, ' ');
 				/* Trigger type */
@@ -4483,7 +4483,7 @@ dumpIndexes(Archive *fout, IndInfo *indinfo, int numIndexes,
 		}
 
 		resetPQExpBuffer(id1);
-		appendPQExpBuffer(id1, fmtId(indinfo[i].indexrelname, force_quotes));
+		appendPQExpBufferStr(id1, fmtId(indinfo[i].indexrelname, force_quotes));
 
 		resetPQExpBuffer(q);
 		appendPQExpBuffer(q, "%s;\n", indinfo[i].indexdef);

Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,7 @@`
`15`	`15`	`*`
`16`	`16`	`*`
`17`	`17`	`* IDENTIFICATION`
`18`		`- * $Header: /cvsroot/pgsql/src/bin/pg_dump/pg_backup_archiver.c,v 1.42 2002/02/11 00:18:20 tgl Exp $`
	`18`	`+ * $Header: /cvsroot/pgsql/src/bin/pg_dump/pg_backup_archiver.c,v 1.42.2.1 2005/04/30 09:08:14 neilc Exp $`
`19`	`19`	`*`
`20`	`20`	`* Modifications - 28-Jun-2000 - pjw@rhyme.com.au`
`21`	`21`	`*`
`@@ -391,7 +391,7 @@ RestoreArchive(Archive AHX, RestoreOptions ropt)`
`391`	`391`	`* mode with libpq.`
`392`	`392`	`*/`
`393`	`393`	`if (te->copyStmt && strlen(te->copyStmt) > 0)`
`394`		`- ahprintf(AH, te->copyStmt);`
	`394`	`+ ahprintf(AH, "%s", te->copyStmt);`
`395`	`395`
`396`	`396`	`(*AH->PrintTocDataPtr) (AH, te, ropt);`
`397`	`397`
`@@ -2006,7 +2006,7 @@ _reconnectAsUser(ArchiveHandle AH, const char dbname, const char *user)`
`2006`	`2006`	`appendPQExpBuffer(qry, " %s\n\n",`
`2007`	`2007`	`fmtId(user, false));`
`2008`	`2008`
`2009`		`- ahprintf(AH, qry->data);`
	`2009`	`+ ahprintf(AH, "%s", qry->data);`
`2010`	`2010`
`2011`	`2011`	`destroyPQExpBuffer(qry);`
`2012`	`2012`	`}`
Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,7 @@`
`22`	`22`	`*`
`23`	`23`	`*`
`24`	`24`	`* IDENTIFICATION`
`25`		`- * $Header: /cvsroot/pgsql/src/bin/pg_dump/pg_dump.c,v 1.241.2.3 2004/03/20 18:12:32 tgl Exp $`
	`25`	`+ * $Header: /cvsroot/pgsql/src/bin/pg_dump/pg_dump.c,v 1.241.2.4 2005/04/30 09:08:14 neilc Exp $`
`26`	`26`	`*`
`27`	`27`	`*-------------------------------------------------------------------------`
`28`	`28`	`*/`
`@@ -436,7 +436,7 @@ dumpClasses_dumpData(Archive fout, char oid, void *dctxv)`
`436`	`436`	`{`
`437`	`437`	`if (field > 0)`
`438`	`438`	`appendPQExpBuffer(q, ",");`
`439`		`- appendPQExpBuffer(q, fmtId(PQfname(res, field), force_quotes));`
	`439`	`+ appendPQExpBufferStr(q, fmtId(PQfname(res, field), force_quotes));`
`440`	`440`	`}`
`441`	`441`	`appendPQExpBuffer(q, ") ");`
`442`	`442`	`archprintf(fout, "%s", q->data);`
`@@ -2599,12 +2599,12 @@ getTables(int numTables, FuncInfo finfo, int numFuncs, const char *tablename)`
`2599`	`2599`	`if (tgisconstraint)`
`2600`	`2600`	`{`
`2601`	`2601`	`appendPQExpBuffer(query, "CREATE CONSTRAINT TRIGGER ");`
`2602`		`- appendPQExpBuffer(query, fmtId(PQgetvalue(res2, i2, i_tgconstrname), force_quotes));`
	`2602`	`+ appendPQExpBufferStr(query, fmtId(PQgetvalue(res2, i2, i_tgconstrname), force_quotes));`
`2603`	`2603`	`}`
`2604`	`2604`	`else`
`2605`	`2605`	`{`
`2606`	`2606`	`appendPQExpBuffer(query, "CREATE TRIGGER ");`
`2607`		`- appendPQExpBuffer(query, fmtId(tgname, force_quotes));`
	`2607`	`+ appendPQExpBufferStr(query, fmtId(tgname, force_quotes));`
`2608`	`2608`	`}`
`2609`	`2609`	`appendPQExpBufferChar(query, ' ');`
`2610`	`2610`	`/* Trigger type */`
`@@ -4483,7 +4483,7 @@ dumpIndexes(Archive fout, IndInfo indinfo, int numIndexes,`
`4483`	`4483`	`}`
`4484`	`4484`
`4485`	`4485`	`resetPQExpBuffer(id1);`
`4486`		`- appendPQExpBuffer(id1, fmtId(indinfo[i].indexrelname, force_quotes));`
	`4486`	`+ appendPQExpBufferStr(id1, fmtId(indinfo[i].indexrelname, force_quotes));`
`4487`	`4487`
`4488`	`4488`	`resetPQExpBuffer(q);`
`4489`	`4489`	`appendPQExpBuffer(q, "%s;\n", indinfo[i].indexdef);`