Bug 30342

Summary: Eliminate the use of String::format
Product: WebKit Reporter: Evan Martin <evan>
Component: TextAssignee: Nobody <webkit-unassigned>
Status: NEW    
Severity: Normal CC: ademar, ap, cmarcelo, darin, jmalonzo, krit, mcatanzaro, nwtour
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=188191
https://bugs.webkit.org/show_bug.cgi?id=37327
https://bugs.webkit.org/show_bug.cgi?id=143773
Bug Depends on: 47664, 47714, 56099, 108883, 151250, 176035, 177028, 187955, 192742, 192746, 194485, 194487, 194752, 194893    
Bug Blocks:    
Attachments:
Description Flags
patch written by Alp Toker evan: review-

Evan Martin
Reported 2009-10-13 14:43:24 PDT
String::format relies internally on printf(), which is in turn affected by the system locale in how it handles decimal separators, etc.
Attachments
patch written by Alp Toker (34.04 KB, patch)
2009-10-13 14:44 PDT, Evan Martin 		evan: review-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Evan Martin
Comment 1 2009-10-13 14:44:46 PDT
Created attachment 41129 [details] patch written by Alp Toker In https://bugs.webkit.org/show_bug.cgi?id=18994 , Darin Adler r-'d this patch, but it is a step in the right direction.
Dirk Schulze
Comment 2 2009-11-18 01:02:35 PST
*** Bug 31514 has been marked as a duplicate of this bug. ***
Dirk Schulze
Comment 3 2009-11-18 01:03:28 PST
*** Bug 18985 has been marked as a duplicate of this bug. ***
Ademar Reis
Comment 4 2010-10-27 14:19:38 PDT
Please don't forget of WebKit2, String::format() is used, for example, in the LocalizationStrategy: WebKit2/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp
Darin Adler
Comment 5 2019-02-03 11:18:13 PST
We’ve reduced the number of call sites using String::format, but there are still quite a few left.
Michael Catanzaro
Comment 6 2019-02-03 12:02:23 PST
Darin, was the dependency on bug #176305 a mistake? It looks unrelated?
Darin Adler
Comment 7 2019-02-03 12:16:27 PST
(In reply to Michael Catanzaro from comment #6) > Darin, was the dependency on bug #176305 a mistake? It looks unrelated? Yes, typo in bug number.
Darin Adler
Comment 8 2019-02-03 12:18:11 PST
(In reply to Darin Adler from comment #7) > (In reply to Michael Catanzaro from comment #6) > > Darin, was the dependency on bug #176305 a mistake? It looks unrelated? > > Yes, typo in bug number. Corrected, it should have been bug 176035.
Darin Adler
Comment 9 2019-02-03 12:26:32 PST
The scope of this bug is too narrow. I think what we really want to do is to eliminate WebKit project use of C library formatting functions in the printf family. Eliminating String::format is a necessary part of this, but not sufficient to cover the whole task. At some point we might want a bug about that.
Michael Catanzaro
Comment 10 2019-02-03 18:27:24 PST
(In reply to Darin Adler from comment #9) > The scope of this bug is too narrow. I think what we really want to do is to > eliminate WebKit project use of C library formatting functions in the printf > family. Eliminating String::format is a necessary part of this, but not > sufficient to cover the whole task. At some point we might want a bug about > that. E.g. our logging functions make heavy use of format specifiers. On Unix they boil down to fprintf(stderr, ...) but it takes some investigation every time I need to discover that. I push a lot of commits changing "%llu" to "%" PRIu64. uint64_t is long long unsigned int on Mac but just long unsigned int on Linux, so using %llu is an error. That's just one example of an annoying difference. Another: Mac has format specifiers that fprintf doesn't accept, e.g. %{private} or something like that. Logging will be a lot more annoying to do without formats, but safer indeed....
Darin Adler
Comment 11 2019-02-25 06:45:22 PST
After r242014 <https://trac.webkit.org/changeset/242014>, String::format itself is now gone. But as Michael mentions above, logging is still using the former implementation of String::format.
Darin Adler
Comment 12 2019-03-01 21:57:39 PST
Relanded in <https://trac.webkit.org/changeset/242308>.
Note You need to log in before you can comment on or make changes to this bug.