Bug 175988

Summary: Add String::format variant that takes va_args
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: Web Template FrameworkAssignee: Eric Carlson <eric.carlson>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, buildbot, cdumez, cmarcelo, commit-queue, darin, dbates, ggaren, 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=176035
Attachments:
Description Flags
Proposed patch.
none
Address post-review feedback. eric.carlson: commit-queue+

Eric Carlson
Reported 2017-08-25 10:32:58 PDT
Add String::format variant that takes va_args so it can be used from a variadic function.
Attachments
Proposed patch. (6.51 KB, patch)
2017-08-25 11:37 PDT, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Address post-review feedback. (1.37 KB, patch)
2017-08-28 08:41 PDT, Eric Carlson 		eric.carlson: commit-queue+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Eric Carlson
Comment 1 2017-08-25 11:37:08 PDT
Created attachment 319091 [details] Proposed patch.
Radar WebKit Bug Importer
Comment 2 2017-08-25 11:37:55 PDT
<rdar://problem/34085521>
WebKit Commit Bot
Comment 3 2017-08-25 13:24:01 PDT
Comment on attachment 319091 [details] Proposed patch. Clearing flags on attachment: 319091 Committed r221203: <http://trac.webkit.org/changeset/221203>
WebKit Commit Bot
Comment 4 2017-08-25 13:24:03 PDT
All reviewed patches have been landed. Closing bug.
Geoffrey Garen
Comment 5 2017-08-25 15:23:04 PDT
Comment on attachment 319091 [details] Proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=319091&action=review > Source/WTF/wtf/text/WTFString.cpp:471 > + va_copy(argsCopy, args); This function calls "va_end(args);" in two places. It also passes "args" to some callees. I think all uses of "args" need to become uses of "argsCopy", no?
Eric Carlson
Comment 6 2017-08-25 16:45:02 PDT
(In reply to Geoffrey Garen from comment #5) > Comment on attachment 319091 [details] > Proposed patch. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=319091&action=review > > > Source/WTF/wtf/text/WTFString.cpp:471 > > + va_copy(argsCopy, args); > > This function calls "va_end(args);" in two places. It also passes "args" to > some callees. I think all uses of "args" need to become uses of "argsCopy", > no? Oops, "va_end(args)" is definitely wrong, but AFAIK the other uses of "args" is correct.
Darin Adler
Comment 7 2017-08-25 20:51:06 PDT
We should stop using String::format.
Darin Adler
Comment 8 2017-08-25 20:54:48 PDT
And instead use something that is type safe. Like use makeString.
Eric Carlson
Comment 9 2017-08-28 08:41:59 PDT
Created attachment 319179 [details] Address post-review feedback.
Eric Carlson
Comment 10 2017-08-28 14:01:55 PDT
(In reply to Darin Adler from comment #8) > And instead use something that is type safe. Like use makeString. Filed bug 176035 for this suggestion.
Note You need to log in before you can comment on or make changes to this bug.