521677 - Fix build warnings: "format not a string literal and no format arguments"

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Daniel Holbert [:dholbert]]

Attachment: fix: s/printf(foo)/printf("%s", foo)/

When building mozilla-central, I get 14 warnings of this type (in various files):

> mozilla/xpcom/base/nsTraceRefcntImpl.cpp:820: warning: format not a string literal and no format arguments

This is GCC detecting a potential format string attack ( http://en.wikipedia.org/wiki/Format_string_attack ).  This is really only an issue when the printed string is user-controlled, but it's trivial to fix, so I don't see why we shouldn't just fix it.

Attached patch fixes 8 instances of this problem in mozilla-central. (The remaining 6 are in NSS.)

Comment 1

[Daniel Holbert [:dholbert]]

I filed bug 521679 on the instances of this warning in NSS code.

Comment 2

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

In some cases these might better use fputs or puts.  Or, in the case of ShowXPCException, combine the two printfs into one.

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: fix v2: use fputs, and use puts in one spot

This patch uses fputs instead of fprintf, for all fprintf calls touched by the previous patch.

I didn't switch all the printf calls to puts, though, because puts always adds a newline[1], and so the switch would change our behavior.

I did replace the |printf(str); printf("\n");| in ShowXPCException to use a single puts call, though.

[1]
http://www.opengroup.org/onlinepubs/009695399/functions/fputs.html#tag_03_182_07
http://www.qnx.com/developers/docs/6.4.1/dinkum_en/ecpp/stdio.html#puts

Comment 4

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Actually, I'd forgotten that puts outputs a newline (whereas fputs doesn't).  So what I'd actually suggest is switching to fputs in almost all cases (with an explicit 'stdout' argument), and continuing to use printf in the one case where you used puts.

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: fix v3: replace printf's with fputs(str, stdout)

Ok -- this version replaces all printfs with fputs(str, stdout), except for the one case in in ShowXPCException, which is now "printf("%s\n", str);" as you suggested in comment 2.

Comment 6

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 407027 [details] [diff] [review]
fix v3: replace printf's with fputs(str, stdout)

r=dbaron.  Thanks for redoing it.

Comment 7

[Daniel Holbert [:dholbert]]

No prob, thanks for the review!
Pushed: http://hg.mozilla.org/mozilla-central/rev/872a2ad2af5d