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

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.

Most of these are harmless (e.g. the ruleutils stuff), but there
is at least one actual bug here: if you create a trigger named
"%sfoo", pg_dump will read uninitialized memory and fail to dump
the trigger correctly.

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index f3458a5abef18e156219ae19ccd361d33206e0c0..aa0ac03357c69063fdfe073cfed99dd5eccde074 100644 (file)
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -3,7 +3,7 @@
  *                             back to source text
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.188 2005/01/13 17:19:10 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.188.4.1 2005/04/30 08:19:44 neilc Exp $
  *
  *       This software is copyrighted by Jan Wieck - Hamburg.
  *
@@ -723,7 +723,7 @@ pg_get_indexdef_worker(Oid indexrelid, int colno, int prettyFlags)
                AttrNumber      attnum = idxrec->indkey[keyno];
 
                if (!colno)
-                       appendStringInfo(&buf, sep);
+                       appendStringInfoString(&buf, sep);
                sep = ", ";
 
                if (attnum != 0)
@@ -1876,7 +1876,7 @@ get_select_query_def(Query *query, deparse_context *context,
                        Oid                     sortcoltype;
                        TypeCacheEntry *typentry;
 
-                       appendStringInfo(buf, sep);
+                       appendStringInfoString(buf, sep);
                        sortexpr = get_rule_sortgroupclause(srt, query->targetList,
                                                                                                force_colno, context);
                        sortcoltype = exprType(sortexpr);
@@ -1945,7 +1945,7 @@ get_basic_select_query(Query *query, deparse_context *context,
                        {
                                SortClause *srt = (SortClause *) lfirst(l);
 
-                               appendStringInfo(buf, sep);
+                               appendStringInfoString(buf, sep);
                                get_rule_sortgroupclause(srt, query->targetList,
                                                                                 false, context);
                                sep = ", ";
@@ -1967,7 +1967,7 @@ get_basic_select_query(Query *query, deparse_context *context,
                if (tle->resdom->resjunk)
                        continue;                       /* ignore junk entries */
 
-               appendStringInfo(buf, sep);
+               appendStringInfoString(buf, sep);
                sep = ", ";
                colno++;
 
@@ -2031,7 +2031,7 @@ get_basic_select_query(Query *query, deparse_context *context,
                {
                        GroupClause *grp = (GroupClause *) lfirst(l);
 
-                       appendStringInfo(buf, sep);
+                       appendStringInfoString(buf, sep);
                        get_rule_sortgroupclause(grp, query->targetList,
                                                                         false, context);
                        sep = ", ";
@@ -2220,7 +2220,7 @@ get_insert_query_def(Query *query, deparse_context *context)
                if (tle->resdom->resjunk)
                        continue;                       /* ignore junk entries */
 
-               appendStringInfo(buf, sep);
+               appendStringInfoString(buf, sep);
                sep = ", ";
 
                /*
@@ -2292,7 +2292,7 @@ get_update_query_def(Query *query, deparse_context *context)
                if (tle->resdom->resjunk)
                        continue;                       /* ignore junk entries */
 
-               appendStringInfo(buf, sep);
+               appendStringInfoString(buf, sep);
                sep = ", ";
 
                /*
@@ -3256,7 +3256,7 @@ get_rule_expr(Node *node, deparse_context *context,
                                        if (tupdesc == NULL ||
                                                !tupdesc->attrs[i]->attisdropped)
                                        {
-                                               appendStringInfo(buf, sep);
+                                               appendStringInfoString(buf, sep);
                                                get_rule_expr(e, context, true);
                                                sep = ", ";
                                        }
@@ -3268,7 +3268,7 @@ get_rule_expr(Node *node, deparse_context *context,
                                        {
                                                if (!tupdesc->attrs[i]->attisdropped)
                                                {
-                                                       appendStringInfo(buf, sep);
+                                                       appendStringInfoString(buf, sep);
                                                        appendStringInfo(buf, "NULL");
                                                        sep = ", ";
                                                }
@@ -3403,7 +3403,7 @@ get_rule_expr(Node *node, deparse_context *context,
                                sep = "";
                                foreach(l, (List *) node)
                                {
-                                       appendStringInfo(buf, sep);
+                                       appendStringInfoString(buf, sep);
                                        get_rule_expr((Node *) lfirst(l), context, showimplicit);
                                        sep = ", ";
                                }

diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index a763a627b0ac0d05e620347bf104a8a56330943f..3f4d5fc9028fce6606fc9b54fe39148a0a49570b 100644 (file)
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -39,7 +39,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  * Portions taken from FreeBSD.
  *
- * $PostgreSQL: pgsql/src/bin/initdb/initdb.c,v 1.73.4.1 2005/01/28 00:36:17 tgl Exp $
+ * $PostgreSQL: pgsql/src/bin/initdb/initdb.c,v 1.73.4.2 2005/04/30 08:19:44 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2610,7 +2610,7 @@ main(int argc, char *argv[])
        make_template0();
 
        if (authwarning != NULL)
-               fprintf(stderr, authwarning);
+               fprintf(stderr, "%s", authwarning);
 
        /* Get directory specification used to start this executable */
        strcpy(bin_dir, argv[0]);

diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index a9ff3482e8849f79184ac5317ce1cbeb49722692..69b7134f320d5c60d5e7144db922b8d4c9d2321d 100644 (file)
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/bin/pg_dump/dumputils.c,v 1.16 2004/12/31 22:03:08 pgsql Exp $
+ * $PostgreSQL: pgsql/src/bin/pg_dump/dumputils.c,v 1.16.4.1 2005/04/30 08:19:44 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -160,7 +160,7 @@ appendStringLiteralDQ(PQExpBuffer buf, const char *str, const char *dqprefix)
        /* start with $ + dqprefix if not NULL */
        appendPQExpBufferChar(delimBuf, '$');
        if (dqprefix)
-               appendPQExpBuffer(delimBuf, dqprefix);
+               appendPQExpBufferStr(delimBuf, dqprefix);
 
        /*
         * Make sure we choose a delimiter which (without the trailing $) is

diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index efa810fdf1ace681971dba3bc4bb7644f1022a46..e814a3cc8a1c51b9ff1055017c480fdd01a80b3c 100644 (file)
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -15,7 +15,7 @@
  *
  *
  * IDENTIFICATION
- *             $PostgreSQL: pgsql/src/bin/pg_dump/pg_backup_archiver.c,v 1.101.4.5 2005/04/15 16:40:59 tgl Exp $
+ *             $PostgreSQL: pgsql/src/bin/pg_dump/pg_backup_archiver.c,v 1.101.4.6 2005/04/30 08:19:44 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -345,7 +345,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);
 
@@ -2197,9 +2197,7 @@ _reconnectToDB(ArchiveHandle *AH, const char *dbname)
 
                appendPQExpBuffer(qry, "\\connect %s\n\n",
                                                  dbname ? fmtId(dbname) : "-");
-
-               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
index 0f088a7ec1d77a81c391c0742f3448449415f572..33c4fa45f98a378aa67bc290b9e1d787e69a1fe6 100644 (file)
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -12,7 +12,7 @@
  *     by PostgreSQL
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/bin/pg_dump/pg_dump.c,v 1.400.4.3 2005/04/15 16:40:59 tgl Exp $
+ *       $PostgreSQL: pgsql/src/bin/pg_dump/pg_dump.c,v 1.400.4.4 2005/04/30 08:19:44 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -975,7 +975,7 @@ dumpTableData_insert(Archive *fout, void *dcontext)
                                {
                                        if (field > 0)
                                                appendPQExpBuffer(q, ", ");
-                                       appendPQExpBuffer(q, fmtId(PQfname(res, field)));
+                                       appendPQExpBufferStr(q, fmtId(PQfname(res, field)));
                                }
                                appendPQExpBuffer(q, ") ");
                                archputs(q->data, fout);
@@ -7439,12 +7439,12 @@ dumpTrigger(Archive *fout, TriggerInfo *tginfo)
        if (tginfo->tgisconstraint)
        {
                appendPQExpBuffer(query, "CREATE CONSTRAINT TRIGGER ");
-               appendPQExpBuffer(query, fmtId(tginfo->tgconstrname));
+               appendPQExpBufferStr(query, fmtId(tginfo->tgconstrname));
        }
        else
        {
                appendPQExpBuffer(query, "CREATE TRIGGER ");
-               appendPQExpBuffer(query, fmtId(tginfo->dobj.name));
+               appendPQExpBufferStr(query, fmtId(tginfo->dobj.name));
        }
        appendPQExpBuffer(query, "\n    ");