Update comparison to cifre #1

Closed
creationix opened this Issue Mar 26, 2013 · 2 comments

Comments

Projects
None yet
2 participants
@creationix

Congrats on writing some nice code. I'm especially impressed that you figured out how to use asm.js correctly and packaged the script to work in a worker.

I only have one nitpick and that is that I feel you've misrepresented my sha1 code in cifre. I only accept binary input, you have to convert ascii strings to binary using the provided conversion tools https://github.com/openpeer/cifre/blob/master/utils.js#L94-L114 which are unicode safe and utf8 encode for all unicode points javascript supports (BMP).

I did have a bug in my code that caused it to give incorrect hashes for inputs over 64 bytes long that I just fixed. hookflash/obsolete.cifre@bd2fbc2

I would love if you included my code in the benchmark to see where it stands and corrected the statement about mine only supporting small ascii strings.

@srijs

This comment has been minimized.

Show comment Hide comment
@srijs

srijs Mar 27, 2013

Owner

Thank you.

Seems like I ran into your bug and then didn't investigate further; I'm sorry for that, I did not mean to misrepresent your work.

I have updated the benchmarks and included a note in the readme that your code works correctly.

Owner

srijs commented Mar 27, 2013

Thank you.

Seems like I ran into your bug and then didn't investigate further; I'm sorry for that, I did not mean to misrepresent your work.

I have updated the benchmarks and included a note in the readme that your code works correctly.

@srijs srijs closed this Mar 27, 2013

@creationix

This comment has been minimized.

Show comment Hide comment
@creationix

creationix Mar 27, 2013

Thanks!

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment