[webkit-reviews] review denied: [Bug 13090] curl (linux/gdk) networking improvements : [Attachment 13653] curl networking improvements

[bugzilla-daemon at webkit.org]

Mark Rowe (bdash) <bdash at webkit.org> has denied Krzysztof Kowalczyk
<kkowalczyk at gmail.com>'s request for review:
Bug 13090: curl (linux/gdk) networking improvements
http://bugs.webkit.org/show_bug.cgi?id=13090

Attachment 13653: curl networking improvements
http://bugs.webkit.org/attachment.cgi?id=13653&action=edit

------- Additional Comments from Mark Rowe (bdash) <bdash at webkit.org>
+    if ((CURLE_OK == err) && ((301 == httpCode) || (302 == httpCode)))
+	 return totalSize;
+

You have unneeded parenthesis here, and in several other places.


+	 CString cstring =  elements[i].m_filename.utf8();

An extra space after the =.

+	 // Fill in the file upload field
+	 curl_formadd(&post,
+		       &lastItem,
+		       CURLFORM_COPYNAME, "sendfile",
+		       CURLFORM_FILE, d->m_fileName,
+		       CURLFORM_END);
+ 
+	 // Fill in the filename field
+	 curl_formadd(&post,
+		       &lastItem,
+		       CURLFORM_COPYNAME, "filename",
+		       CURLFORM_COPYCONTENTS, d->m_fileName,
+		       CURLFORM_END);
+ 
+	 // Fill in the submit field too, even if this is rarely needed
+	 curl_formadd(&post,
+		       &lastItem,
+		       CURLFORM_COPYNAME, "submit",
+		       CURLFORM_COPYCONTENTS, "send",
+		       CURLFORM_END);

This looks to be based on curl example code.  It seems incorrect to be adding a
form field named "submit" for each file that is uploaded.  I think the
indentation style of this code block could be cleaned up a great deal too.


+	 if (job == node->job()) {
+	     node->setRemoved(true);
+	     return true;
+	     node = next;
+	 }

I think "node = next;" should be outside the braces.


+class ResourceHandleList
+{

Brace should be on same line as class statement.