Bug 58579 - Presence of vsnprintf is not depending on compiler, but on windows C runtime used
Summary: Presence of vsnprintf is not depending on compiler, but on windows C runtime ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-14 14:53 PDT by Fridrich Strba
Modified: 2011-05-04 15:56 PDT (History)
5 users (show)

See Also:

Attachments
Patch (1.51 KB, patch)
2011-04-14 14:57 PDT, Fridrich Strba 		no flags Details | Formatted Diff | Diff
Modified patch: instead of OS(WINDOWS) test, add just COMPILER(MINGW) to that branch (1.78 KB, patch)
2011-04-15 00:00 PDT, Fridrich Strba 		no flags Details | Formatted Diff | Diff
A new version of patch: removing accidental paste of unrelated garbage in the Source/WebCore/dom/XMLDocumentParserLibxml2.cpp (1.47 KB, patch)
2011-04-15 00:06 PDT, Fridrich Strba 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Fridrich Strba 2011-04-14 14:53:04 PDT 
The vsnprintf is part of Microsoft C runtime. So, don't condition its use on COMPILER(MSVC), but on OS(WINDOWS)
Comment 1 Fridrich Strba 2011-04-14 14:57:23 PDT 
Created attachment 89661 [details]
Patch
Comment 2 Alexey Proskuryakov 2011-04-14 19:05:24 PDT 
As you say, the presence of vsnprintf depends on C runtime - so why is OS(WINDOWS) appropriate?
Comment 3 Fridrich Strba 2011-04-14 21:40:21 PDT 
(In reply to comment #2)
> As you say, the presence of vsnprintf depends on C runtime - so why is OS(WINDOWS) appropriate?
Because the only relevant compilers out there and used by Webkit (MSVC and MinGW) link against the Microsoft C runtime
Comment 4 Fridrich Strba 2011-04-15 00:00:48 PDT 
Created attachment 89744 [details]
Modified patch: instead of OS(WINDOWS) test, add just COMPILER(MINGW) to that branch
Comment 5 Fridrich Strba 2011-04-15 00:03:56 PDT 
(In reply to comment #2)
> As you say, the presence of vsnprintf depends on C runtime - so why is OS(WINDOWS) appropriate?

I modified the patch so that it only adds the COMPILER(MINGW) to the test. On a second thought, one can imagine a Windows compiler that comes with its own C runtime that is different from the Microsoft one.
Comment 6 WebKit Review Bot 2011-04-15 00:05:04 PDT 
Attachment 89744 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8452070
Comment 7 Fridrich Strba 2011-04-15 00:06:43 PDT 
Created attachment 89745 [details]
A new version of patch: removing accidental paste of unrelated garbage in the Source/WebCore/dom/XMLDocumentParserLibxml2.cpp
Comment 8 Build Bot 2011-04-15 00:22:47 PDT 
Attachment 89744 [details] did not build on win:
Build output: http://queues.webkit.org/results/8452076
Comment 9 Adam Barth 2011-04-17 01:38:03 PDT 
Comment on attachment 89745 [details]
A new version of patch: removing accidental paste of unrelated garbage in the Source/WebCore/dom/XMLDocumentParserLibxml2.cpp

Ok.
Comment 10 Fridrich Strba 2011-05-03 02:24:37 PDT 
This one has a positive review. Anybody can put a nice "+" to the commit-queue too?
Comment 11 Xan Lopez 2011-05-04 13:45:47 PDT 
Comment on attachment 89745 [details]
A new version of patch: removing accidental paste of unrelated garbage in the Source/WebCore/dom/XMLDocumentParserLibxml2.cpp

cq+
Comment 12 WebKit Commit Bot 2011-05-04 15:56:03 PDT 
Comment on attachment 89745 [details]
A new version of patch: removing accidental paste of unrelated garbage in the Source/WebCore/dom/XMLDocumentParserLibxml2.cpp

Clearing flags on attachment: 89745

Committed r85800: <http://trac.webkit.org/changeset/85800>
Comment 13 WebKit Commit Bot 2011-05-04 15:56:09 PDT 
All reviewed patches have been landed.  Closing bug.