Bug 176035

Summary: Logger should use makeString instead of String::format
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: WebCore Misc.Assignee: Eric Carlson <eric.carlson>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, buildbot, commit-queue, darin, jer.noble, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=175988
Bug Depends on:    
Bug Blocks: 30342    
Attachments:
Description Flags
Proposed patch.
none
Proposed patch.
none
Proposed patch.
jer.noble: review+
Patch for landing. none

Eric Carlson
Reported 2017-08-28 14:01:14 PDT
As noted in bug 175988, String::format is not type safe so Logger should use makeString instead.
Attachments
Proposed patch. (10.22 KB, patch)
2017-08-28 14:27 PDT, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Proposed patch. (10.22 KB, patch)
2017-08-28 14:33 PDT, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Proposed patch. (10.27 KB, patch)
2017-08-28 14:54 PDT, Eric Carlson 		jer.noble: review+
DetailsFormatted DiffDiff
Patch for landing. (10.26 KB, patch)
2017-08-28 19:04 PDT, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Eric Carlson
Comment 1 2017-08-28 14:27:04 PDT
Created attachment 319207 [details] Proposed patch.
Build Bot
Comment 2 2017-08-28 14:29:07 PDT
Attachment 319207 [details] did not pass style-queue: ERROR: Source/WebCore/PAL/pal/Logger.h:39: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WebCore/PAL/pal/Logger.h:137: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WebCore/PAL/pal/Logger.h:138: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] Total errors found: 3 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Eric Carlson
Comment 3 2017-08-28 14:33:45 PDT
Created attachment 319208 [details] Proposed patch.
Eric Carlson
Comment 4 2017-08-28 14:54:12 PDT
Created attachment 319212 [details] Proposed patch.
Alex Christensen
Comment 5 2017-08-28 16:06:15 PDT
Comment on attachment 319212 [details] Proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=319212&action=review > Source/WebCore/PAL/pal/Logger.h:40 > + template<typename U = T> static typename std::enable_if<std::is_same<U, int>::value, String>::type toString(int argument) { return String::number(argument); } This could be slightly abbreviated if you used std::enable_if_t instead of std::enable_if<...>::type
Eric Carlson
Comment 6 2017-08-28 19:04:29 PDT
Created attachment 319227 [details] Patch for landing.
WebKit Commit Bot
Comment 7 2017-08-28 19:58:52 PDT
The commit-queue encountered the following flaky tests while processing attachment 319227 [details]: http/tests/security/cross-origin-xsl-BLOCKED.html bug 51054 (authors: abarth@webkit.org, jochen@chromium.org, and rniwa@webkit.org) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 8 2017-08-28 19:59:22 PDT
Comment on attachment 319227 [details] Patch for landing. Clearing flags on attachment: 319227 Committed r221288: <http://trac.webkit.org/changeset/221288>
Darin Adler
Comment 9 2019-02-03 12:24:25 PST
Looks like this was resolved more than a year ago.
Radar WebKit Bug Importer
Comment 10 2019-02-03 12:25:27 PST
<rdar://problem/47773077>
Note You need to log in before you can comment on or make changes to this bug.