[webkit-reviews] review denied: [Bug 170270] [GCrypt] Implement PBKDF2 support : [Attachment 305947] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Apr 1 17:17:30 PDT 2017

Michael Catanzaro <mcatanzaro at igalia.com> has denied  review:
Bug 170270: [GCrypt] Implement PBKDF2 support

Attachment 305947: Patch


--- Comment #7 from Michael Catanzaro <mcatanzaro at igalia.com> ---
Comment on attachment 305947
  --> https://bugs.webkit.org/attachment.cgi?id=305947

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

Actually, I'd like to look at this one again.

> Source/WebCore/crypto/gcrypt/CryptoAlgorithmPBKDF2GCrypt.cpp:64
> +    Vector<uint8_t> result(length / 8);

Can you explain why the length of the result is only 1/8th the size of the
length of the input? Seems a bit arbitrary?

> Source/WebCore/crypto/gcrypt/CryptoAlgorithmPBKDF2GCrypt.cpp:71
> +    return WTFMove(result);

Hm, no this is not right, you can't use WTFMove when returning a local
variable... it's stack-allocated and has been destroyed, right...? Isn't this
undefined behavior or something otherwise bad? Evens if I'm wrong about that,
you're guaranteed move semantics if you just return result directly anyway, so
you should do that instead, no need for WTFMove here.

More information about the webkit-reviews mailing list