[webkit-reviews] review denied: [Bug 46259] [Qt] QNetworkReplyHandler forces buffered output for FormData with files : [Attachment 68360] intial proposal

[bugzilla-daemon at webkit.org]

Andreas Kling <andreas.kling at nokia.com> has denied Sriram Neelakandan
<sriram.neelakandan at gmail.com>'s request for review:
Bug 46259: [Qt] QNetworkReplyHandler forces buffered output for FormData with
files
https://bugs.webkit.org/show_bug.cgi?id=46259

Attachment 68360: intial proposal 
https://bugs.webkit.org/attachment.cgi?id=68360&action=review

------- Additional Comments from Andreas Kling <andreas.kling at nokia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=68360&action=review

r- for compilation-breaking typo and missing ChangeLog entry.
Please see http://trac.webkit.org/wiki/QtWebKitContrib for information on how
to make patches for WebKit, specifically the "Submitting a patch" section.

> QNetworkReplyHandler.h.new:100
> +    qint64 getFormDataSize() { return m_totalSize; }

This method should be const.

> QNetworkReplyHandler.cpp.new:70
> +    m_totalSize=computeSize();

Coding style, should be "m_totalSize = computeSize();"

> QNetworkReplyHandler.cpp.new:81
> +    for (int i=0; i<m_formElements.size(); i++)

Coding style, should be "for (int i = 0; i < m_formElements.size(); ++i)"

> QNetworkReplyHandler.cpp.new:84
> +	   if (element.m_type == FormDataElement::data) {

Coding style, shouldn't have {

> QNetworkReplyHandler.cpp.new:85
> +	       m_dataSize+=element.m_data.size();

Coding style, should be "m_dataSize += element.m_data.size();"

> QNetworkReplyHandler.cpp.new:88
> +	       m_fileSize+=fi.size();

Coding style, should be "m_fileSize += fi.size();"

> QNetworkReplyHandler.cpp.new:94
> +    return (m_dataSize + m_fileSize);

Unnecessary parentheses.

> QNetworkReplyHandler.cpp.new:482
> +	       //We may be uploading files so prevent QNR from buffering data

Coding style, please leave a space after //

> QNetworkReplyHandler.cpp.new:483
> +	       m_request.setHeader(QNetworkRequest::ContentLengthHeader,
posdDevice->getFormDataSize());

Typo, "posdDevice" should be "postDevice"

> QNetworkReplyHandler.cpp.new:494
> +	       //We may be uploading files so prevent QNR from buffering data

Coding style, please leave a space after //

> QNetworkReplyHandler.cpp.new:495
> +	       m_request.setHeader(QNetworkRequest::ContentLengthHeader,
posdDevice->getFormDataSize());

Typo, "posdDevice" should be "postDevice"