datrs / hypercore

Secure, distributed, append-only log
https://docs.rs/hypercore
Apache License 2.0
327 stars 37 forks source link

refactor binary interface types from `usize` to their specific sizes #70

Closed FreddieRidell closed 5 years ago

FreddieRidell commented 5 years ago

Feature Request

Summary

In some places usize is being used in situations where it would be better to explicitly define what bit size we want to use. One example can be found here: https://github.com/datrs/hypercore/blob/c7c8757959123f524d99e90d3c4f471542e022a0/src/storage/node.rs#L53-L54

Motivation

The crate is currently built with a 64 bit word size in mind, this will cause issues when run on a 32-bit system

Explanation

Review all uses of usize, evaluate if u64 would be more appropriate, change the types, fix any compiler errors that occour. (optional: build and run on a 32 bit system to check compatability)

Unresolved Questions

Is this something that should be done now? are there valid reasons to postpone this work? Are there any other relevant issues that have not been described in this ticket

yoshuawuyts commented 5 years ago

@FreddieRidell I fully agree with you here, and would welcome patches to change this. We've done this in a few other crates too already. There shouldn't be any blockers to landing such changes.

FreddieRidell commented 5 years ago

I've put up a PR, I had a look through the codebase and this was the only place where we obviously should use u64 over usize.

Unfortunately it seems there's no way to easily test 32-bit on travis-ci, and I don't have any way to test it available.