| Summary: | [BREWMP] Port vprintf_stderr_common | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Kwang Yul Seo <skyul> | ||||||||||||||
| Component: | Web Template Framework | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | beergun, commit-queue, eric, joybro201, tkent, webkit.review.bot, xhiloh | ||||||||||||||
| Priority: | P2 | ||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||
| Hardware: | Other | ||||||||||||||||
| OS: | Other | ||||||||||||||||
| Bug Depends on: | |||||||||||||||||
| Bug Blocks: | 33564 | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
Kwang Yul Seo
2010-01-12 18:27:04 PST
Created attachment 46419 [details]
Port vprintf_stderr_common to BREW
Attachment 46419 [details] did not pass style-queue:
Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
JavaScriptCore/wtf/Assertions.cpp:113: Use 0 instead of NULL. [readability/null] [4]
JavaScriptCore/wtf/Assertions.cpp:115: Use 0 instead of NULL. [readability/null] [5]
Total errors found: 2
Created attachment 47256 [details]
Port vprintf_stderr_common
Use PLATFORM(BREWMP).
Comment on attachment 47256 [details]
Port vprintf_stderr_common
OK, but lets fix Assertions.h for the COMPILE_ASSERT problem.
If you're mallocing locally, you should use an OwnPtr. Or maybe a Vector<char> here.
Created attachment 47479 [details]
Port vprintf_stderr_common
Remove #undef COMPILE_ASSERT and use Vector<char>.
Attachment 47479 [details] did not pass style-queue:
Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
JavaScriptCore/wtf/Assertions.cpp:112: Use 0 instead of NULL. [readability/null] [4]
JavaScriptCore/wtf/Assertions.cpp:114: Use 0 instead of NULL. [readability/null] [5]
Total errors found: 2
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 47480 [details]
Port vprintf_stderr_common
Fix style errors.
Attachment 47480 [details] did not pass style-queue:
Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
JavaScriptCore/wtf/Assertions.cpp:112: Use 0 instead of NULL. [readability/null] [4]
Total errors found: 1
If any of these errors are false positives, please file a bug against check-webkit-style.
It seems that check-webkit-style complains about NULL in the comment. Created attachment 47481 [details]
Port vprintf_stderr_common
Replace NULL with 0 in the comment.
Comment on attachment 47481 [details]
Port vprintf_stderr_common
I probably would have made the while loop its own function which took a Vector<char>& and knew how to walk the vector spitting out chunks to the debug stream.
This should probably just be "static":
123 char printBuffer[printBufferSize + 1];
Can VSNPRINTF ever return < 0? If so, then this code can deadlock.
I guess I would have used buffer.size() to bound my while loop instead of the return value from VSNPRINTF. That way you know for sure that you never walk off the end of the Vector.
Created attachment 60397 [details]
Patch
Follow Eric's advice.
Ping. Comment on attachment 60397 [details]
Patch
Looks ok.
Comment on attachment 60397 [details] Patch Clearing flags on attachment: 60397 Committed r66345: <http://trac.webkit.org/changeset/66345> All reviewed patches have been landed. Closing bug. |