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

Randomness Component is Broken in Struct Header #11

Open
ChrisBove opened this issue Jul 13, 2021 · 0 comments
Open

Randomness Component is Broken in Struct Header #11

ChrisBove opened this issue Jul 13, 2021 · 0 comments

Comments

@ChrisBove
Copy link

Issue Description

As mentioned in b93d569#r43494808, the changes to EncodeEntropyRand in #7 cause the random component to be 0 when the function is called on a ulid. The problem occurs with how casting is happening here:

inline void EncodeEntropyRand(ULID& ulid) {
	ulid.data[6] = (uint8_t)(std::rand() * 255ull) / RAND_MAX;

In C++ order of operations, a c style cast happens before multiplication or division. That means this expression is resolved:

 (std::rand() * 255ull)

then cast as a uint8_t before the division by RAND_MAX occurs. Thus, you end up with a number always less than 0 (since a unint8_t can only hold up to 255, and RAND_MAX (on 32 bit platforms) is 2147483647).

Fix

A fix was proposed in #9. However, I think that only capturing the last 0xFF part of the output of rand() may be suboptimal (see std::rand() notes here).

Another fix would be to force the correct order of operations and cast the entire expression. I've tested that this is working.

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

1 participant