Bug 175988 – Add String::format variant that takes va_args

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Eric Carlson]

Add String::format variant that takes va_args so it can be used from a variadic function.

[Eric Carlson]

Created attachment 319091 [details]
Proposed patch.

[Radar WebKit Bug Importer]

<rdar://problem/34085521>

[WebKit Commit Bot]

Comment on attachment 319091 [details]
Proposed patch.

Clearing flags on attachment: 319091

Committed r221203: <http://trac.webkit.org/changeset/221203>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Geoffrey Garen]

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]

(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]

We should stop using String::format.

[Darin Adler]

And instead use something that is type safe. Like use makeString.

[Eric Carlson]

Created attachment 319179 [details]
Address post-review feedback.

[Eric Carlson]

(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.