[webkit-reviews] review denied: [Bug 59830] Add WTFPrintf() : [Attachment 91756] Update patch to include PRINTF macro, based on feedback from Dan.

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied Jeff Miller <jeffm at apple.com>'s
request for review:
Bug 59830: Add WTFPrintf()
https://bugs.webkit.org/show_bug.cgi?id=59830

Attachment 91756: Update patch to include PRINTF macro, based on feedback from
Dan.
https://bugs.webkit.org/attachment.cgi?id=91756&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=91756&action=review

OK, I'm fine with adding PRINTF. I think that I have too many comments to
justify another quick review round though.

> Source/JavaScriptCore/wtf/Assertions.cpp:299
> +    if (format[strlen(format) - 1] != '\n')

What if format is zero length?

> Source/JavaScriptCore/wtf/Assertions.h:363
> +#define PRINTF() ((void)0)

Perhaps #define PRINTF printf ?

> Source/JavaScriptCore/wtf/Assertions.h:365
> +#define PRINTF(arg...) ((void)0)

Ditto.

> Source/JavaScriptCore/wtf/Assertions.h:367
> +#elif LOG_DISABLED
> +#define PRINTF(...) ((void)0)

I don't think that LOG_DISABLED should affect PRINTF. In fact, printf debugging
is very useful in release builds, too.