[webkit-reviews] review denied: [Bug 78449] [WebSocket] Add deflater/inflater classes : [Attachment 126931] Patch

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied Kenichi Ishibashi
<bashi at chromium.org>'s request for review:
Bug 78449: [WebSocket] Add deflater/inflater classes
https://bugs.webkit.org/show_bug.cgi?id=78449

Attachment 126931: Patch
https://bugs.webkit.org/attachment.cgi?id=126931&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=126931&action=review


> Source/WebCore/websockets/WebSocketDeflater.cpp:59
> +    ASSERT(m_windowBits >= 8 && m_windowBits <= 15);

Please split this into two ASSERTs.

ASSERT(m_windowBits >= 8);
ASSERT(m_windowBits <= 15);

> Source/WebCore/websockets/WebSocketDeflater.cpp:86
> +    m_stream.next_in = reinterpret_cast<Bytef*>(const_cast<char*>(data));
> +    m_stream.avail_in = length;
> +    m_stream.next_out = reinterpret_cast<Bytef*>(m_buffer.data() +
writePosition);
> +    m_stream.avail_out = maxLength;

Repeating reinterpret_cast<> and const_cast<> is not good.
Please add a helper function like setStreamParameters(data, length,
m_buffer.data() + writePosition, maxLength);

> Source/WebCore/websockets/WebSocketDeflater.cpp:104
> +	   m_stream.next_in = reinterpret_cast<Bytef*>(0);
> +	   m_stream.avail_in = 0;
> +	   m_stream.next_out = reinterpret_cast<Bytef*>(m_buffer.data() +
writePosition);
> +	   m_stream.avail_out = availableCapacity;

ditto.

> Source/WebCore/websockets/WebSocketDeflater.cpp:163
> +	   m_stream.next_in = reinterpret_cast<Bytef*>(const_cast<char*>(data)
+ consumedSoFar);
> +	   m_stream.avail_in = remainingLength;
> +	   m_stream.next_out = reinterpret_cast<Bytef*>(m_buffer.data() +
writePosition);
> +	   m_stream.avail_out = availableCapacity;

ditto.

> Source/WebCore/websockets/WebSocketDeflater.cpp:191
> +	   m_stream.next_in = strippedFields + consumedSoFar;
> +	   m_stream.avail_in = remainingLength;
> +	   m_stream.next_out = reinterpret_cast<Bytef*>(m_buffer.data() +
writePosition);
> +	   m_stream.avail_out = availableCapacity;

ditto.