pierrec / xxHash

Pure Go implementation of xxHash (32 and 64 bits versions)
BSD 3-Clause "New" or "Revised" License
58 stars 17 forks source link

[FIXED] Windows binary #7

Closed sergeevabc closed 5 years ago

sergeevabc commented 5 years ago

Dear Pierre, Could you be so kind to generate .exe for the rest of us who are mere Windows users w/o compiler?

pierrec commented 5 years ago

Hello,

Thank you for the suggestion, I have added binaries to release v0.1.2.

sergeevabc commented 5 years ago

Thank you for a quick reply, Pierre. While looking for xxHash32 output I was surprised to find that -mode 0 produces unnecessary lines, i.e.

$ printf "hello" | xxhsum -mode 0
write 606290984 2246822519 0 1640531535 <---
sum32 606290984 2246822519 0 1640531535 <---
f97700fb

Also, there is something wrong with hash itself, I expected to see fb0077f9.

pierrec commented 5 years ago

Sorry, didnt test it thoroughy. Have a go with 0.1.4 please.

sergeevabc commented 5 years ago

Great! 0.1.4 works as expected, Pierre.

sergeevabc commented 5 years ago
Usage of xxhash:
  -mode int
        hash mode: 0=32bits, 1=64bits (default 1)
  -seed uint
        seed value

@pierrec, -seed seems to be optionally waiting for uint. This notation is kinda ambiguous, because uint is expected to be integer between 0 and 4294967295, whereas you use what is called uint64, so the upper limit is much greater. Perhaps you should mention both the accepted range (0–18446744073709551615) and the default value which is 0.

pierrec commented 5 years ago

In Go, uint is either 32 or 64 bits depending on the platform. In this case, it actually depends on the xxhash mode, which is what I have reflected in the updated cli message. Thanks for added clarity.

sergeevabc commented 5 years ago

@pierrec, hmm… the following works, however max seed with 32-bit mode is expected to be 4294967295.

printf "hello" | xxhsum_v0.1.5.exe -mode 0 -seed 18446744073709551615
2711808b stdin
pierrec commented 5 years ago

Yes, 18446744073709551615 gets truncated to 4294967295.

sergeevabc commented 5 years ago

Covert truncation under the hood? This is not fair to the end-user. It is me and you who are aware of allowed ranges, but others might type long enough seed believing the very seed will be used, whereas in fact it would be truncated to another value. Consider adding a validation step:

xxh32: 0–4294967295
xxh64: 0–18446744073709551615
pierrec commented 5 years ago

Not under cover, I just dont expect an uint64 to necessarily fit into an uint32. What do you suggest then? Print out a warning or give an error and abort?