[webkit-reviews] review denied: [Bug 56331] FileSystemWin.cpp needs listDirectory() implementation : [Attachment 85724] [PATCH] Fix v2

[bugzilla-daemon at webkit.org]

Adam Roben (:aroben) <aroben at apple.com> has denied Brian Weinstein
<bweinstein at apple.com>'s request for review:
Bug 56331: FileSystemWin.cpp needs listDirectory() implementation
https://bugs.webkit.org/show_bug.cgi?id=56331

Attachment 85724: [PATCH] Fix v2
https://bugs.webkit.org/attachment.cgi?id=85724&action=review

------- Additional Comments from Adam Roben (:aroben) <aroben at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=85724&action=review

I think the StringConcatenate changes need some work.

> Source/JavaScriptCore/wtf/text/StringConcatenate.h:102
> +	   if (m_length > std::numeric_limits<unsigned>::max())
> +	       CRASH();

I don't think this will ever be true.

> Source/JavaScriptCore/wtf/text/StringConcatenate.h:112
> +	   for (unsigned i = 0; i < m_length; ++i) {
> +	       unsigned char c = m_buffer[i];
> +	       destination[i] = c;
> +	   }

You're stripping off the high 8 bits of each character. That doesn't seem
right. I think memcpy would be better.

> Source/WebCore/platform/win/PathWalker.h:27
> +#include <wtf/text/WTFString.h>

Why is this neeed? Would a forward declaration suffice?