indygreg added a comment.
This seems mostly good to me.
INLINE COMMENTS
> cext.c:68
> + if (SHA1DCFinal(hash_out, &ctx)) {
> + PyErr_SetString(PyExc_OverflowError,
> + "sha1 collision attack detected");
I'm not super keen on overloading `OverflowError` here. Or is this how
`hashlib` works in the Python standard library?
> cext.c:92
> + static const char hexdigit[] = "0123456789abcdef";
> + for (int i = 0; i < 20; ++i) {
> + hexhash[i * 2] = hexdigit[hash[i] >> 4];
I didn't verify this is correct because I didn't want to think too hard. Test
coverage for parity with `hashlib` would be appreciated.
> cext.c:107
> + }
> + clone->ctx = self->ctx;
> + return (PyObject *)clone;
Should this be a `memcpy` or some such? Or is this opaque type safe to copy by
value? Test coverage for this demonstrating that a seeded hasher which is
copied can properly diverge would be appreciated.
> cext.c:172
> + sha1ctxType.tp_new = PyType_GenericNew;
> + if (PyType_Ready(&sha1ctxType) < 0)
> + return;
Nit: please always use braces.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D7815/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D7815
To: durin42, #hg-reviewers
Cc: indygreg, spectral, mjpieters, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
