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-

Description Evan Martin 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.
Comment 1 Evan Martin 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.
Comment 2 Dirk Schulze 2009-11-18 01:02:35 PST
*** Bug 31514 has been marked as a duplicate of this bug. ***
Comment 3 Dirk Schulze 2009-11-18 01:03:28 PST
*** Bug 18985 has been marked as a duplicate of this bug. ***
Comment 4 Ademar Reis 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
Comment 5 Darin Adler 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.
Comment 6 Michael Catanzaro 2019-02-03 12:02:23 PST
Darin, was the dependency on bug #176305 a mistake? It looks unrelated?
Comment 7 Darin Adler 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.
Comment 8 Darin Adler 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.
Comment 9 Darin Adler 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.
Comment 10 Michael Catanzaro 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....
Comment 11 Darin Adler 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.
Comment 12 Darin Adler 2019-03-01 21:57:39 PST
Relanded in <https://trac.webkit.org/changeset/242308>.