[webkit-reviews] review denied: [Bug 17971] [Curl] FormData processing should be moved to its own class : [Attachment 19904] First patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Julien Chaffraix
<julien.chaffraix at gmail.com>'s request for review:
Bug 17971: [Curl] FormData processing should be moved to its own class
http://bugs.webkit.org/show_bug.cgi?id=17971

Attachment 19904: First patch
http://bugs.webkit.org/attachment.cgi?id=19904&action=edit

------- Additional Comments from Eric Seidel <eric at webkit.org>
Looks fine to me.  My only question was if the ownership was right for
ResourceHandle*, i.e. why is it safe to hold a weak pointer?  Using FILE*
directly kinda sucks too.  But I guess we don't have any nice C++ wrapper for
it.

+    size_t toSend = size * nmemb;

integer overflow?  We really should have some sort of safe-multiply function.

What the heck does "nmemb" stand for?  A more clear variable name would be
helpful here.

You put * next to the argument name in at least two places.  This is in
violation of our style guidelines for C++:
+    size_t read(void *ptr, size_t size, size_t nmemb);


In general looks great.  I was going to r+ it,	but I think there are enough
little nits to warrent a second round.