[PATCHES] format string cleanup

Neil Conway Fri, 29 Apr 2005 01:22:05 -0700

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 stuff, when a function takes a format string and the caller passes 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 unitialized memory and likely not dump the trigger correctly:

=> create trigger "%sxyz_trig" before insert on xyz for each row execute procedure xyz();

yields the following pg_dump output:

CREATE TRIGGER ""%sxyz_trig"xyz_trig"
    BEFORE INSERT ON xyz
    FOR EACH ROW
    EXECUTE PROCEDURE xyz();

Barring any objections, I'll apply this to HEAD and backport it to stable branches back to 7.2 tomorrow.

-Neil

Index: src/backend/utils/adt/ruleutils.c
===================================================================
RCS file: /var/lib/cvs/pgsql/src/backend/utils/adt/ruleutils.c,v
retrieving revision 1.193
diff -c -r1.193 ruleutils.c
*** src/backend/utils/adt/ruleutils.c	14 Apr 2005 20:03:26 -0000	1.193
--- src/backend/utils/adt/ruleutils.c	29 Apr 2005 07:36:32 -0000
***************
*** 733,739 ****
  		AttrNumber	attnum = idxrec->indkey.values[keyno];
  
  		if (!colno)
! 			appendStringInfo(&buf, sep);
  		sep = ", ";
  
  		if (attnum != 0)
--- 733,739 ----
  		AttrNumber	attnum = idxrec->indkey.values[keyno];
  
  		if (!colno)
! 			appendStringInfoString(&buf, sep);
  		sep = ", ";
  
  		if (attnum != 0)
***************
*** 1885,1891 ****
  			Oid			sortcoltype;
  			TypeCacheEntry *typentry;
  
! 			appendStringInfo(buf, sep);
  			sortexpr = get_rule_sortgroupclause(srt, query->targetList,
  												force_colno, context);
  			sortcoltype = exprType(sortexpr);
--- 1885,1891 ----
  			Oid			sortcoltype;
  			TypeCacheEntry *typentry;
  
! 			appendStringInfoString(buf, sep);
  			sortexpr = get_rule_sortgroupclause(srt, query->targetList,
  												force_colno, context);
  			sortcoltype = exprType(sortexpr);
***************
*** 1954,1960 ****
  			{
  				SortClause *srt = (SortClause *) lfirst(l);
  
! 				appendStringInfo(buf, sep);
  				get_rule_sortgroupclause(srt, query->targetList,
  										 false, context);
  				sep = ", ";
--- 1954,1960 ----
  			{
  				SortClause *srt = (SortClause *) lfirst(l);
  
! 				appendStringInfoString(buf, sep);
  				get_rule_sortgroupclause(srt, query->targetList,
  										 false, context);
  				sep = ", ";
***************
*** 1976,1982 ****
  		if (tle->resjunk)
  			continue;			/* ignore junk entries */
  
! 		appendStringInfo(buf, sep);
  		sep = ", ";
  		colno++;
  
--- 1976,1982 ----
  		if (tle->resjunk)
  			continue;			/* ignore junk entries */
  
! 		appendStringInfoString(buf, sep);
  		sep = ", ";
  		colno++;
  
***************
*** 2040,2046 ****
  		{
  			GroupClause *grp = (GroupClause *) lfirst(l);
  
! 			appendStringInfo(buf, sep);
  			get_rule_sortgroupclause(grp, query->targetList,
  									 false, context);
  			sep = ", ";
--- 2040,2046 ----
  		{
  			GroupClause *grp = (GroupClause *) lfirst(l);
  
! 			appendStringInfoString(buf, sep);
  			get_rule_sortgroupclause(grp, query->targetList,
  									 false, context);
  			sep = ", ";
***************
*** 2229,2235 ****
  		if (tle->resjunk)
  			continue;			/* ignore junk entries */
  
! 		appendStringInfo(buf, sep);
  		sep = ", ";
  
  		/*
--- 2229,2235 ----
  		if (tle->resjunk)
  			continue;			/* ignore junk entries */
  
! 		appendStringInfoString(buf, sep);
  		sep = ", ";
  
  		/*
***************
*** 2301,2307 ****
  		if (tle->resjunk)
  			continue;			/* ignore junk entries */
  
! 		appendStringInfo(buf, sep);
  		sep = ", ";
  
  		/*
--- 2301,2307 ----
  		if (tle->resjunk)
  			continue;			/* ignore junk entries */
  
! 		appendStringInfoString(buf, sep);
  		sep = ", ";
  
  		/*
***************
*** 3268,3274 ****
  					if (tupdesc == NULL ||
  						!tupdesc->attrs[i]->attisdropped)
  					{
! 						appendStringInfo(buf, sep);
  						get_rule_expr(e, context, true);
  						sep = ", ";
  					}
--- 3268,3274 ----
  					if (tupdesc == NULL ||
  						!tupdesc->attrs[i]->attisdropped)
  					{
! 						appendStringInfoString(buf, sep);
  						get_rule_expr(e, context, true);
  						sep = ", ";
  					}
***************
*** 3280,3286 ****
  					{
  						if (!tupdesc->attrs[i]->attisdropped)
  						{
! 							appendStringInfo(buf, sep);
  							appendStringInfo(buf, "NULL");
  							sep = ", ";
  						}
--- 3280,3286 ----
  					{
  						if (!tupdesc->attrs[i]->attisdropped)
  						{
! 							appendStringInfoString(buf, sep);
  							appendStringInfo(buf, "NULL");
  							sep = ", ";
  						}
***************
*** 3415,3421 ****
  				sep = "";
  				foreach(l, (List *) node)
  				{
! 					appendStringInfo(buf, sep);
  					get_rule_expr((Node *) lfirst(l), context, showimplicit);
  					sep = ", ";
  				}
--- 3415,3421 ----
  				sep = "";
  				foreach(l, (List *) node)
  				{
! 					appendStringInfoString(buf, sep);
  					get_rule_expr((Node *) lfirst(l), context, showimplicit);
  					sep = ", ";
  				}
Index: src/bin/initdb/initdb.c
===================================================================
RCS file: /var/lib/cvs/pgsql/src/bin/initdb/initdb.c,v
retrieving revision 1.82
diff -c -r1.82 initdb.c
*** src/bin/initdb/initdb.c	28 Apr 2005 21:47:16 -0000	1.82
--- src/bin/initdb/initdb.c	29 Apr 2005 07:24:30 -0000
***************
*** 2609,2615 ****
  	make_template0();
  
  	if (authwarning != NULL)
! 		fprintf(stderr, authwarning);
  
  	/* Get directory specification used to start this executable */
  	strcpy(bin_dir, argv[0]);
--- 2609,2615 ----
  	make_template0();
  
  	if (authwarning != NULL)
! 		fprintf(stderr, "%s", authwarning);
  
  	/* Get directory specification used to start this executable */
  	strcpy(bin_dir, argv[0]);
Index: src/bin/pg_dump/dumputils.c
===================================================================
RCS file: /var/lib/cvs/pgsql/src/bin/pg_dump/dumputils.c,v
retrieving revision 1.16
diff -c -r1.16 dumputils.c
*** src/bin/pg_dump/dumputils.c	31 Dec 2004 22:03:08 -0000	1.16
--- src/bin/pg_dump/dumputils.c	29 Apr 2005 07:12:25 -0000
***************
*** 160,166 ****
  	/* start with $ + dqprefix if not NULL */
  	appendPQExpBufferChar(delimBuf, '$');
  	if (dqprefix)
! 		appendPQExpBuffer(delimBuf, dqprefix);
  
  	/*
  	 * Make sure we choose a delimiter which (without the trailing $) is
--- 160,166 ----
  	/* start with $ + dqprefix if not NULL */
  	appendPQExpBufferChar(delimBuf, '$');
  	if (dqprefix)
! 		appendPQExpBufferStr(delimBuf, dqprefix);
  
  	/*
  	 * Make sure we choose a delimiter which (without the trailing $) is
Index: src/bin/pg_dump/pg_backup_archiver.c
===================================================================
RCS file: /var/lib/cvs/pgsql/src/bin/pg_dump/pg_backup_archiver.c,v
retrieving revision 1.107
diff -c -r1.107 pg_backup_archiver.c
*** src/bin/pg_dump/pg_backup_archiver.c	15 Apr 2005 16:40:36 -0000	1.107
--- src/bin/pg_dump/pg_backup_archiver.c	29 Apr 2005 07:14:11 -0000
***************
*** 345,351 ****
  						 * mode with libpq.
  						 */
  						if (te->copyStmt && strlen(te->copyStmt) > 0)
! 							ahprintf(AH, te->copyStmt);
  
  						(*AH->PrintTocDataPtr) (AH, te, ropt);
  
--- 345,351 ----
  						 * mode with libpq.
  						 */
  						if (te->copyStmt && strlen(te->copyStmt) > 0)
! 							ahprintf(AH, "%s", te->copyStmt);
  
  						(*AH->PrintTocDataPtr) (AH, te, ropt);
  
***************
*** 2197,2205 ****
  
  		appendPQExpBuffer(qry, "\\connect %s\n\n",
  						  dbname ? fmtId(dbname) : "-");
! 
! 		ahprintf(AH, qry->data);
! 
  		destroyPQExpBuffer(qry);
  	}
  
--- 2197,2203 ----
  
  		appendPQExpBuffer(qry, "\\connect %s\n\n",
  						  dbname ? fmtId(dbname) : "-");
! 		ahprintf(AH, "%s", qry->data);
  		destroyPQExpBuffer(qry);
  	}
  
Index: src/bin/pg_dump/pg_dump.c
===================================================================
RCS file: /var/lib/cvs/pgsql/src/bin/pg_dump/pg_dump.c,v
retrieving revision 1.407
diff -c -r1.407 pg_dump.c
*** src/bin/pg_dump/pg_dump.c	15 Apr 2005 16:40:36 -0000	1.407
--- src/bin/pg_dump/pg_dump.c	29 Apr 2005 07:25:35 -0000
***************
*** 976,982 ****
  				{
  					if (field > 0)
  						appendPQExpBuffer(q, ", ");
! 					appendPQExpBuffer(q, fmtId(PQfname(res, field)));
  				}
  				appendPQExpBuffer(q, ") ");
  				archputs(q->data, fout);
--- 976,982 ----
  				{
  					if (field > 0)
  						appendPQExpBuffer(q, ", ");
! 					appendPQExpBufferStr(q, fmtId(PQfname(res, field)));
  				}
  				appendPQExpBuffer(q, ") ");
  				archputs(q->data, fout);
***************
*** 7599,7610 ****
  	if (tginfo->tgisconstraint)
  	{
  		appendPQExpBuffer(query, "CREATE CONSTRAINT TRIGGER ");
! 		appendPQExpBuffer(query, fmtId(tginfo->tgconstrname));
  	}
  	else
  	{
  		appendPQExpBuffer(query, "CREATE TRIGGER ");
! 		appendPQExpBuffer(query, fmtId(tginfo->dobj.name));
  	}
  	appendPQExpBuffer(query, "\n    ");
  
--- 7599,7610 ----
  	if (tginfo->tgisconstraint)
  	{
  		appendPQExpBuffer(query, "CREATE CONSTRAINT TRIGGER ");
! 		appendPQExpBufferStr(query, fmtId(tginfo->tgconstrname));
  	}
  	else
  	{
  		appendPQExpBuffer(query, "CREATE TRIGGER ");
! 		appendPQExpBufferStr(query, fmtId(tginfo->dobj.name));
  	}
  	appendPQExpBuffer(query, "\n    ");

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?


               http://www.postgresql.org/docs/faq

[PATCHES] format string cleanup

Reply via email to