Bug 41553 - Make StringExtras.h versions of snprintf and vsnprintf match the unix versions
Summary: Make StringExtras.h versions of snprintf and vsnprintf match the unix versions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-02 17:48 PDT by Sam Weinig
Modified: 2010-07-03 17:11 PDT (History)
2 users (show)

See Also:


Attachments
Patch (3.80 KB, patch)
2010-07-02 17:50 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff
Updated patch (4.14 KB, patch)
2010-07-02 18:05 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch 3 (4.10 KB, patch)
2010-07-03 14:14 PDT, Sam Weinig
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2010-07-02 17:48:00 PDT
Make StringExtras.h versions of snprintf and vsnprintf match the unix versions.  MSVC does not ensure the buffers are null terminated as the unix versions do.
Comment 1 Sam Weinig 2010-07-02 17:50:45 PDT
Created attachment 60424 [details]
Patch
Comment 2 Darin Adler 2010-07-02 17:53:39 PDT
Comment on attachment 60424 [details]
Patch

> +    // In the case where the string entirely filled the buffer, _vsnprintf will not
> +    // null-terminate it, but vsnprintf must.
> +    if (count > 0)
> +        buffer[count - 1] = '\0';

But what about the case where Windows *does* have vsnprintf, but it has the wrong behavior and does not terminate the buffer with a nul character? As I understand it, this is the case with MSVC 8 and newer!
Comment 3 Sam Weinig 2010-07-02 18:05:24 PDT
Created attachment 60426 [details]
Updated patch
Comment 4 WebKit Review Bot 2010-07-02 18:06:41 PDT
Attachment 60426 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
JavaScriptCore/wtf/StringExtras.h:56:  wtf_vsnprintf is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 WebKit Review Bot 2010-07-03 03:16:38 PDT
Attachment 60426 [details] did not build on win:
Build output: http://webkit-commit-queue.appspot.com/results/3399146
Comment 6 Darin Adler 2010-07-03 10:05:17 PDT
Comment on attachment 60426 [details]
Updated patch

> +// Work around a bug in Microsoft's implementation of vsnprintf, where 
> +// vsnprintf does not null terminate the buffer
> +#define vsnprintf(x, y) wtf_vsnprintf(x, y)

vsnprintf has 4 arguments, not 2, which is presumably why the bot is failing to build
Comment 7 Sam Weinig 2010-07-03 14:14:28 PDT
Created attachment 60457 [details]
Patch 3
Comment 8 WebKit Review Bot 2010-07-03 14:17:29 PDT
Attachment 60457 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
JavaScriptCore/wtf/StringExtras.h:56:  wtf_vsnprintf is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Sam Weinig 2010-07-03 17:11:53 PDT
Landed in r62457.