Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support 32 bits #465

Closed
samoht opened this issue Jul 21, 2017 · 3 comments
Closed

Support 32 bits #465

samoht opened this issue Jul 21, 2017 · 3 comments

Comments

@samoht
Copy link
Member

samoht commented Jul 21, 2017

See mirage/mirage-www#562 (comment)

It seems that the recent commits to speed-up hashing broke 32 bits support. We should test this (and fix the issue, probably by removing support for int in Irmin.Type)

@samoht
Copy link
Member Author

samoht commented Jul 26, 2017

@hannesm why do you think these 2 lines might break something? It seems to me that we always try to serialise to 64b, whatever the platform is, which is the right way to have a portable binary serialization. WDYT?

@hannesm
Copy link
Member

hannesm commented Jul 26, 2017

I said might in another thread since Int64.to_int (actually done in https://github.com/mirage/irmin/pull/458/files#diff-0b43dc6466ffe5ba4e4629f71c33631eR898) sounds very unsafe to me. I don't have sufficient insight into the code base to analyse whether there is an actual problem. Also I'm not sure when this code is executed, and with what input (controlled by whom?), and whether the goals include dumping on 64bit and restoring on 32bit.

On 64bit, the Int64.to_int is also "unsafe": Int64.to_int Int64.max_int -> -1

@samoht
Copy link
Member Author

samoht commented Jul 27, 2017

See #469

@samoht samoht closed this as completed Jul 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants