mirrors/git-annex
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions

code review

Browse source
This commit is contained in:
Joey Hess 2022-12-26 13:09:40 -04:00
parent efda811404
commit 53a40ca40f
No known key found for this signature in database
GPG key ID: DB12DB0FF05F8F38
1 changed files with 30 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

30
doc/bugs/blake3_hash_support/comment_4_3ef2af8eb62a81299e49a3b7dccd337e._comment Normal file
View file

@ -0,0 +1,30 @@
[[!comment format=mdwn
username="joey"
subject="""comment 4"""
date="2022-12-26T16:26:45Z"
content="""
I've only vendored libraries that I developed myself,
that made sense to split out of git-annex. I don't want to get into
vendoring other stuff here. But I think a build flag is ok, it does not
need vendoring.
Looking at the code, I suspect that updateIncrementalVerifier
might have a space leak. It does not force the S.length to be evaluated.
And BLAKE3.update might also be lazy, I am not sure?
Compare with the very similar code in mkIncrementalHasher,
which is careful to force the length and also the hash update.
Also I would prefer not to use Control.Arrow just because I'm not
idiomatically familiar with it. Not that `***` is very complicated,
but it obfuscates the code slightly for me.
So, I would be happier if that part of the code were changed to just
spell it all out and force everything like mkIncrementalHasher does.
(Or if mkIncrementalHasher were generalized so that it could reuse its
code. Looks doable by parameterizing hashUpdate and hashFinalize.)
Also doc/backends.mdwn will need to be updated.
The rest of the code looks good and I'm ready to merge it once the above
are addressed.
"""]]