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:

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	Details Formatted Diff Diff
Proposed patch. (10.22 KB, patch) 2017-08-28 14:33 PDT, Eric Carlson	no flags	Details Formatted Diff Diff
Proposed patch. (10.27 KB, patch) 2017-08-28 14:54 PDT, Eric Carlson	jer.noble: review+	Details Formatted Diff Diff
Patch for landing. (10.26 KB, patch) 2017-08-28 19:04 PDT, Eric Carlson	no flags	Details Formatted Diff Diff
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.