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 37f3687688ced4da02f4a59f267f441fd878a5c4..d2f51de3c911b5cf1499d3b9c091f8574861fd3a 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.193 2005/04/14 20:03:26 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.194 2005/04/30 08:08:50 neilc Exp $
  *
  *   This software is copyrighted by Jan Wieck - Hamburg.
  *
@@ -733,7 +733,7 @@ pg_get_indexdef_worker(Oid indexrelid, int colno, int prettyFlags)
        AttrNumber  attnum = idxrec->indkey.values[keyno];
 
        if (!colno)
-           appendStringInfo(&buf, sep);
+           appendStringInfoString(&buf, sep);
        sep = ", ";
 
        if (attnum != 0)
@@ -1885,7 +1885,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);
@@ -1954,7 +1954,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 = ", ";
@@ -1976,7 +1976,7 @@ get_basic_select_query(Query *query, deparse_context *context,
        if (tle->resjunk)
            continue;           /* ignore junk entries */
 
-       appendStringInfo(buf, sep);
+       appendStringInfoString(buf, sep);
        sep = ", ";
        colno++;
 
@@ -2040,7 +2040,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 = ", ";
@@ -2229,7 +2229,7 @@ get_insert_query_def(Query *query, deparse_context *context)
        if (tle->resjunk)
            continue;           /* ignore junk entries */
 
-       appendStringInfo(buf, sep);
+       appendStringInfoString(buf, sep);
        sep = ", ";
 
        /*
@@ -2301,7 +2301,7 @@ get_update_query_def(Query *query, deparse_context *context)
        if (tle->resjunk)
            continue;           /* ignore junk entries */
 
-       appendStringInfo(buf, sep);
+       appendStringInfoString(buf, sep);
        sep = ", ";
 
        /*
@@ -3268,7 +3268,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 = ", ";
                    }
@@ -3280,7 +3280,7 @@ get_rule_expr(Node *node, deparse_context *context,
                    {
                        if (!tupdesc->attrs[i]->attisdropped)
                        {
-                           appendStringInfo(buf, sep);
+                           appendStringInfoString(buf, sep);
                            appendStringInfo(buf, "NULL");
                            sep = ", ";
                        }
@@ -3415,7 +3415,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 7d424749d624e6e4e6f0c4e3c2685283c40de582..f4951c3f1bbdc6d44cfa81ab694820d76aeeaa06 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.82 2005/04/28 21:47:16 tgl Exp $
+ * $PostgreSQL: pgsql/src/bin/initdb/initdb.c,v 1.83 2005/04/30 08:08:51 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2609,7 +2609,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..f3cece132d1b86e1e9f95edfba455fc1ebac4c03 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.17 2005/04/30 08:08:51 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 36852157b47c7308d9816142052bfe8299898986..85368ca6abbce3eeb554a43e430131243ae14ec3 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.107 2005/04/15 16:40:36 tgl Exp $
+ *     $PostgreSQL: pgsql/src/bin/pg_dump/pg_backup_archiver.c,v 1.108 2005/04/30 08:08:51 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 9737dd378626814b66c2269e9cbd4638cd79f372..60a492e2bed2d7538e4607b6f675253eda01fb75 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.407 2005/04/15 16:40:36 tgl Exp $
+ *   $PostgreSQL: pgsql/src/bin/pg_dump/pg_dump.c,v 1.408 2005/04/30 08:08:51 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -976,7 +976,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);
@@ -7599,12 +7599,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    ");