[webkit-reviews] review canceled: [Bug 55039] Add SHA-1 for new WebSocket protocol : [Attachment 83473] Patch v1

[bugzilla-daemon at webkit.org]

Yuta Kitamura <yutak at chromium.org> has canceled Yuta Kitamura
<yutak at chromium.org>'s request for review:
Bug 55039: Add SHA-1 for new WebSocket protocol
https://bugs.webkit.org/show_bug.cgi?id=55039

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

------- Additional Comments from Yuta Kitamura <yutak at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=83473&action=review

>> Source/JavaScriptCore/wtf/SHA1.cpp:105
>> +static inline uint32_t s(int n, uint32_t x)
> 
> We can call this as rotateLeft or something like this?

Yes, I will rename.

>> Source/JavaScriptCore/wtf/SHA1.cpp:186
>> +	for (int t = 0; t < 80; ++t) {
> 
> I guess most implementations split this loop into 4 for-loop for performance.
I don't know if performance is important here though.

Performance is not an important factor at least for WebSocket protocol
implementation (a short message will be hashed for each WebSocket handshake).
I'd like to keep the code simple for now... I can certainly tune up the
implementation when a performance problem comes up.

>> Source/JavaScriptCore/wtf/SHA1.h:60
>> +	size_t m_totalBytes; // Number of bytes added so far.
> 
> Won't we need to calculate sha1 of >4GB stream?

Will change this line to "uint64_t m_totalBytes".