[webkit-reviews] review denied: [Bug 14206] MSVC7 support and PLATFORM define fixes for Windows : [Attachment 16561] New patch with changes made

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Oct 6 23:21:28 PDT 2007

Adam Roben <aroben at apple.com> has denied Kevin Ollivier
<kevino at theolliviers.com>'s request for review:
Bug 14206: MSVC7 support and PLATFORM define fixes for Windows

Attachment 16561: New patch with changes made

------- Additional Comments from Adam Roben <aroben at apple.com>
 #include <stdint.h>
 #include <stdlib.h>
+#include <stdarg.h>

These #include statements should be sorted alphabetically.

+// MSVC8 and above define this function
+inline int vsnprintf(char *str, size_t size, const char* format, ...) 
+    int result;
+    va_list args;
+    va_start(args, format);
+    result = _vsnprintf(str, size, format, args);
+    va_end(args);
+    return result;

I don't think it's a good idea to define this function if it's line-for-line
identical to the snprintf function defined just above it. Can we just do
#define vsnprintf snprintf?

 #ifndef WINVER

     if (IsDebuggerPresent()) {

I'm not sure this is the right check to make. For instance, I don't think
Qt/Win wants to be calling IsDebuggerPresent (or even links against the
necessary library to obtain this symbol). Perhaps this should be
COMPILER(MSVC)? I'm not completely sure though.

 #include <kjs/identifier.h>
 #include <wtf/Vector.h>
+#include <wtf/StringExtras.h>

Please keep the #includes sorted.

 #include <wtf/Platform.h>
 #include <wtf/Vector.h>
+#include <wtf/StringExtras.h>


r- for now so that we can fix the vsnprintf and WIN_OS issues.

More information about the webkit-reviews mailing list