Bug 213204 – Consistently use WTF_ATTRIBUTE_PRINTF in Assertions.[cpp,h]

• Component: Web Template Framework
• Status: RESOLVED FIXED

[Michael Catanzaro]

r262831 removed the GCC pragma that was suppressing these warnings:

[70/1715] Building CXX object Source/WTF/wtf/CMakeFiles/WTF.dir/Assertions.cpp.o
/home/mcatanzaro/Projects/WebKit/Source/WTF/wtf/Assertions.cpp: In function ‘void vprintf_stderr_with_prefix(const char*, const char*, __va_list_tag*)’:
/home/mcatanzaro/Projects/WebKit/Source/WTF/wtf/Assertions.cpp:186:56: warning: function ‘void vprintf_stderr_with_prefix(const char*, const char*, __va_list_tag*)’ might be a candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
  186 |     vprintf_stderr_common(formatWithPrefix.data(), args);
      |                                                        ^
/home/mcatanzaro/Projects/WebKit/Source/WTF/wtf/Assertions.cpp: In function ‘void vprintf_stderr_with_trailing_newline(const char*, __va_list_tag*)’:
/home/mcatanzaro/Projects/WebKit/Source/WTF/wtf/Assertions.cpp:193:43: warning: function ‘void vprintf_stderr_with_trailing_newline(const char*, __va_list_tag*)’ might be a candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
  193 |         vprintf_stderr_common(format, args);
      |                                           ^
/home/mcatanzaro/Projects/WebKit/Source/WTF/wtf/Assertions.cpp:202:57: warning: function ‘void vprintf_stderr_with_trailing_newline(const char*, __va_list_tag*)’ might be a candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
  202 |     vprintf_stderr_common(formatWithNewline.data(), args);
      |                                                         ^
/home/mcatanzaro/Projects/WebKit/Source/WTF/wtf/Assertions.cpp: In function ‘void WTFLogVaList(WTFLogChannel*, const char*, __va_list_tag*)’:
/home/mcatanzaro/Projects/WebKit/Source/WTF/wtf/Assertions.cpp:435:74: warning: function ‘void WTFLogVaList(WTFLogChannel*, const char*, __va_list_tag*)’ might be a candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
  435 |     String loggingString = WTF::createWithFormatAndArguments(format, args);
      | 

Solution is to use WTF_ATTRIBUTE_PRINTF where recommended. According to https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes, the first argument to WTF_ATTRIBUTE_PRINTF should be the position of the format string itself (starting from 1), and the second argument should be the position of the ... parameter pack (or 0  for functions that take va_list).

[Michael Catanzaro]

Created attachment 401919 [details]
Patch

[Darin Adler]

Comment on attachment 401919 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=401919&action=review

I didn’t set the commit queue flag yet because I’d like EWS to finish first.

> Source/WTF/wtf/Assertions.cpp:175
>  ALLOW_NONLITERAL_FORMAT_BEGIN

I wonder if we still need this?

[Michael Catanzaro]

Comment on attachment 401919 [details]
Patch

EWS are looking green....

(In reply to Darin Adler from comment #2)
> > Source/WTF/wtf/Assertions.cpp:175
> >  ALLOW_NONLITERAL_FORMAT_BEGIN
> 
> I wonder if we still need this?

I'll check. Guess: probably yes.

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #3)
> I'll check. Guess: probably yes.

I was wrong, it doesn't seem to be needed with GCC 10, at least I don't see any new warnings. I can't vouch for other compilers, though.

[EWS]

Committed r263062: <https://trac.webkit.org/changeset/263062>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 401919 [details].

[Radar WebKit Bug Importer]

<rdar://problem/64380332>