[webkit-reviews] review granted: [Bug 171059] LayoutTests crypto/subtle/ecdsa-generate-key-sign-verify-p384.html and crypto/subtle/ecdsa-generate-key-sign-verify-p256.html are flaky failures : [Attachment 308011] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has granted Jiewen Tan
<jiewen_tan at apple.com>'s request for review:
Bug 171059: LayoutTests crypto/subtle/ecdsa-generate-key-sign-verify-p384.html
and crypto/subtle/ecdsa-generate-key-sign-verify-p256.html are flaky failures
https://bugs.webkit.org/show_bug.cgi?id=171059

Attachment 308011: Patch

https://bugs.webkit.org/attachment.cgi?id=308011&action=review




--- Comment #6 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 308011
  --> https://bugs.webkit.org/attachment.cgi?id=308011
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=308011&action=review

Looks fine, but I think you might want to clean up the code a little. r=me with
cleanups tp signECDSA.

> Source/WebCore/crypto/mac/CryptoAlgorithmECDSAMac.cpp:92
> +	   size_t pos = newSignature.size();

Isn't pos always zero here, since you only reserved capacity above, and did not
actually fill the vector with anything?

> Source/WebCore/crypto/mac/CryptoAlgorithmECDSAMac.cpp:97
> +    // Otherwise skipping the leading 0s of r.

// Otherwise skip the leading 0s of r.

> Source/WebCore/crypto/mac/CryptoAlgorithmECDSAMac.cpp:98
> +    if (signature[offset] > keyLengthInBytes)

Should this be "else if"?

> Source/WebCore/crypto/mac/CryptoAlgorithmECDSAMac.cpp:113
> +    // Otherwise skipping the leading 0s of s.

"// Otherwise skip the leading 0s of s."