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

Invalid memory unit conversions #204

Closed
1993alexey opened this issue Oct 22, 2024 · 2 comments · Fixed by #205
Closed

Invalid memory unit conversions #204

1993alexey opened this issue Oct 22, 2024 · 2 comments · Fixed by #205

Comments

@1993alexey
Copy link

1993alexey commented Oct 22, 2024

I was writing unit tests for my functions that rely on this awesome library and found a serious problem with memory unit conversions.

Here is a complete code example

const { bytes, Measure, tebi } = require('safe-units')
console.log(Measure.of(1, tebi(bytes)).in(bytes))

The result was 256 B which is wrong. It should have been 1099511627776 B

After investigating this problem, I can see that the left shift operation doesn't produce the desired results due to overflow. It results in a 32 bit integer.

export const kibi: PrefixFn = Measure.prefix("Ki", 1 << 10); // 1024
export const mebi = Measure.prefix("Mi", 1 << 20); // 1048576
export const gibi = Measure.prefix("Gi", 1 << 30); // 1073741824
export const tebi = Measure.prefix("Ti", 1 << 40); // 256
export const pibi = Measure.prefix("Pi", 1 << 50); // 262144
export const exbi = Measure.prefix("Ei", 1 << 60); // 268435456
export const zebi = Measure.prefix("Zi", 1 << 70); // 64
export const yobi = Measure.prefix("Yi", 1 << 80); // 65536
@jscheiny
Copy link
Owner

Thanks for catching this, put a fix up here: #205

@1993alexey
Copy link
Author

@jscheiny Thanks for the quick fix. When are you planning to release this change to NPM?

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

Successfully merging a pull request may close this issue.

2 participants