The channel is passed around only to check its state. Clean this up by adding a method to check whether a log channel is enabled.
Created attachment 226723 [details] patch
Comment on attachment 226723 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=226723&action=review > Source/WebCore/platform/Logging.cpp:50 > + WTFLogChannel* channel = WTFLogChannelByName(logChannels, logChannelCount, name.utf8().data()); Would be good to check the channel is not null.
Created attachment 226901 [details] patch (In reply to comment #2) > (From update of attachment 226723 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=226723&action=review > > > Source/WebCore/platform/Logging.cpp:50 > > + WTFLogChannel* channel = WTFLogChannelByName(logChannels, logChannelCount, name.utf8().data()); > > Would be good to check the channel is not null. Added null check.
Attachment 226901 [details] did not pass style-queue: ERROR: Source/WebCore/platform/Logging.h:83: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/WebCore/platform/Logging.cpp:53: Else clause should never be on same line as else (use 2 lines) [whitespace/newline] [4] Total errors found: 2 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 226902 [details] patch - fixed style (In reply to comment #4) > Attachment 226901 [details] did not pass style-queue: > > > ERROR: Source/WebCore/platform/Logging.h:83: Code inside a namespace should not be indented. [whitespace/indent] [4] I'm not quite sure about this style error. The exactly same change in this file passed a few days ago. Someone upgraded the stlye checker? :) Should I fix the indentation in the whole file, even the lines not touched by this patch? > ERROR: Source/WebCore/platform/Logging.cpp:53: Else clause should never be on same line as else (use 2 lines) [whitespace/newline] [4] Fixed.
Attachment 226902 [details] did not pass style-queue: ERROR: Source/WebCore/platform/Logging.h:83: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 226902 [details] patch - fixed style View in context: https://bugs.webkit.org/attachment.cgi?id=226902&action=review This is basically good but the logChannelEnabled local variable can easily be avoided, we like early returns in WebKit :) > Source/WebCore/platform/Logging.cpp:53 > + if (channel) This can be transformed to an early return: if (!channel) return false; and then below return channel->state != WTFLogChannelOff; So no need for a local variable
Created attachment 227168 [details] patch (In reply to comment #7) > (From update of attachment 226902 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=226902&action=review > > This is basically good but the logChannelEnabled local variable can easily be avoided, we like early returns in WebKit :) Thank you for the correction, it looks nicer indeed :) > > > Source/WebCore/platform/Logging.cpp:53 > > + if (channel) > > This can be transformed to an early return: if (!channel) return false; > > and then below return channel->state != WTFLogChannelOff; > So no need for a local variable
Attachment 227168 [details] did not pass style-queue: ERROR: Source/WebCore/platform/Logging.h:83: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 227168 [details] patch Clearing flags on attachment: 227168 Committed r165955: <http://trac.webkit.org/changeset/165955>
All reviewed patches have been landed. Closing bug.