[webkit-reviews] review granted: [Bug 171213] [GCrypt] CryptoKeyRSA: implement create(), keySizeInBits(), buildAlgorithm(), exportData() : [Attachment 307961] Patch

[bugzilla-daemon at webkit.org]

Michael Catanzaro <mcatanzaro at igalia.com> has granted Zan Dobersek
<zan at falconsigh.net>'s request for review:
Bug 171213: [GCrypt] CryptoKeyRSA: implement create(), keySizeInBits(),
buildAlgorithm(), exportData()
https://bugs.webkit.org/show_bug.cgi?id=171213

Attachment 307961: Patch

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




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

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

Looks good. Please wait for Jiewen before committing, as usual.

> Source/WebCore/crypto/gcrypt/CryptoKeyRSAGCrypt.cpp:337
> +	   // dp -- d mod (p - 1)

Thank you for the comments. ;)

> Source/WebCore/crypto/gcrypt/CryptoKeyRSAGCrypt.cpp:341
> +	       gcry_mpi_sub_ui(pm1MPI, pm1MPI, 1);

Do you think it would be safer to use an extra local variable for the result
here (i.e. have both pMPI and pm1MPI locals)? I'm sure this is fine now and
probably will be forever, but it could be disastrous if future versions of
GCrypt can't handle the first parameter being the same as a subsequent
parameter. That's probably paranoid though.