[webkit-reviews] review denied: [Bug 177360] [GCrypt] Only report libgcrypt errors when logging is enabled : [Attachment 321535] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Sep 22 06:29:11 PDT 2017


Michael Catanzaro <mcatanzaro at igalia.com> has denied  review:
Bug 177360: [GCrypt] Only report libgcrypt errors when logging is enabled
https://bugs.webkit.org/show_bug.cgi?id=177360

Attachment 321535: Patch

https://bugs.webkit.org/attachment.cgi?id=321535&action=review




--- Comment #2 from Michael Catanzaro <mcatanzaro at igalia.com> ---
Comment on attachment 321535
  --> https://bugs.webkit.org/attachment.cgi?id=321535
Patch

I would expect errors to always be printed to stdout regardless of whether
logging is enabled or not. What sort of error spam are you seeing that makes it
preferable to silence all errors from libgcrypt?

I don't think using LOG_DISABLED to guard a call to WTFLogAlways makes sense,
since WTFLogAlways is what you use to log something even when logs are
disabled. You could use ENABLE(DEVELOPER_MODE), but even that doesn't make
sense since there's no fundamental reason why errors should be silenced in
release mode but not developer mode.

If you really need to silence these errors, I think the right way would be to
define a new log channel in Logging.h for WebCrypto. The problem is that's
complicated by the fact that we have this weird transitional split between
WebCore/platform and PAL right now, where PAL can't use WebCore/platform. I
think PAL is causing more problems than it solves so long as we have it left in
this transitional state. I guess PAL needs its own Logging.h.

But logging is really intended for debug: errors should print always unless you
have a *really* good reason not to.


More information about the webkit-reviews mailing list