Skip to content
This repository has been archived by the owner on Jun 27, 2023. It is now read-only.

Fix/handle overflow #53

Merged
merged 3 commits into from
Dec 21, 2018
Merged

Fix/handle overflow #53

merged 3 commits into from
Dec 21, 2018

Conversation

Stebalien
Copy link
Member

  1. Fix an overflow where we can run out of hash bits.
  2. Keep all the hash bits even if we don't use them. I can't see a reason to use murmur64 over murmur128, murmur64 is just a truncated (non-standard) murmur128.

If we have a deep enough directory, we could walk off the end of the bit array.
1. It's less confusing. Murmur64 is something our library supports by truncating
a Murmur128 hash.
2. We'll only use the part we need anyways.
directory trees.
@ghost ghost assigned Stebalien Dec 21, 2018
@ghost ghost added the status/in-progress In progress label Dec 21, 2018
hamt/hamt.go Outdated
idx := hv.Next(ds.tableSizeLg2)
idx, err := hv.Next(ds.tableSizeLg2)
if err != nil {
return fmt.Errorf("sharded directory too deep")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're ignoring the returned error

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I returned a different error from hashBits.Next because, technically, it isn't specific to sharded directories. However, yeah, that probably wasn't worth the confusion (fixed).

I didn't do this before because this datastructure *technically* isn't specific
to sharded directories but, really, that was just even more confusing.
@Stebalien Stebalien merged commit d7bb120 into master Dec 21, 2018
@ghost ghost removed the status/in-progress In progress label Dec 21, 2018
@Stebalien Stebalien deleted the fix/handle-overflow branch December 21, 2018 03:29
Jorropo pushed a commit to Jorropo/go-libipfs that referenced this pull request Jan 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants