Refactor the code to handle missing va_end() call after va_start() in String::format

• Component: Web Template Framework
• Status: RESOLVED FIXED

[Santosh Mahto]

In WTFString.cpp  WTF::String::format(), va_end call could be missed after va_start() is called if the function return
at following places.

if (result == 0)
     return String("");
if (result < 0)
    return String();

[Santosh Mahto]

Created attachment 245445 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 245445 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=245445&action=review

> Source/WTF/wtf/text/WTFString.cpp:469
>      // Not calling va_end/va_start here happens to work on lots of
>      // systems, but fails e.g. on 64bit Linux.

The patch looks good to me, but it needs to remove the orphaned comment.

[Santosh Mahto]

Created attachment 245539 [details]
Patch

[Santosh Mahto]

(In reply to comment #2)
> Comment on attachment 245445 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=245445&action=review
> 
> > Source/WTF/wtf/text/WTFString.cpp:469
> >      // Not calling va_end/va_start here happens to work on lots of
> >      // systems, but fails e.g. on 64bit Linux.
> 
> The patch looks good to me, but it needs to remove the orphaned comment.

Thanks for review. please review the next update incorporating your suggestion.

[Alexey Proskuryakov]

Comment on attachment 245539 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=245539&action=review

> Source/WTF/ChangeLog:8
> +        Moved va_start/va_end call to match  seperately.

The correct spelling is "separately". Also, not sure if this comment adds anything of value on top of the title.

Is there any known benefit, other than the code looking slightly more nicely (e.g. does that fix some platform)? It would be good to explain that in the bug for posterity if so.

[Santosh Mahto]

Created attachment 245631 [details]
Patch for landing

[Santosh Mahto]

(In reply to comment #5)
> Comment on attachment 245539 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=245539&action=review
> 
> > Source/WTF/ChangeLog:8
> > +        Moved va_start/va_end call to match  seperately.
> 
> The correct spelling is "separately". Also, not sure if this comment adds
> anything of value on top of the title.
> 
> Is there any known benefit, other than the code looking slightly more nicely
> (e.g. does that fix some platform)? It would be good to explain that in the
> bug for posterity if s

I updated the comment section.may be it make sense now a bit. sending it to land

[WebKit Commit Bot]

Comment on attachment 245631 [details]
Patch for landing

Clearing flags on attachment: 245631

Committed r179354: <http://trac.webkit.org/changeset/179354>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.