Skip to content

Conversation

@JohnnyCena123
Copy link
Contributor

No description provided.

@manuel5975p
Copy link
Contributor

Just a minor suggestion, maybe pick more "unique" names for the macros otherwise it might affect other code and emit "already defined" warnings.

@raysan5
Copy link
Owner

raysan5 commented Oct 13, 2025

@JohnnyCena123 thanks for the addition, added some details to be reviewed

@raysan5
Copy link
Owner

raysan5 commented Oct 13, 2025

@manuel5975p That's a good point, also macros can be undefined at the end of the function.

@CrackedPixel

This comment was marked as resolved.

@JohnnyCena123
Copy link
Contributor Author

JohnnyCena123 commented Oct 13, 2025

@JohnnyCena123 im getting incorrect results when i try this. can you show a minimal main.c that tests this?

already have 2 ready yeah
sha256-buffer.c
sha256-stream.c

@CrackedPixel

This comment was marked as resolved.

@raysan5
Copy link
Owner

raysan5 commented Oct 14, 2025

@CrackedPixel Did you review this implementation against @manuel5975p one? Are the results as expected?

Out of curiosity, do both implementations have a similar performance?

src/rcore.c Outdated
hash[6] = 0x1f83d9ab;
hash[7] = 0x5be0cd19;

unsigned long long const bitLen = dataSize*8;
Copy link
Contributor

Choose a reason for hiding this comment

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

This causes undefined behaviour for valid values of dataSize because both are signed integers (dataSize and the 8 literal)

@JohnnyCena123
Copy link
Contributor Author

there have been some updates

@raysan5 raysan5 merged commit 17bc628 into raysan5:master Oct 15, 2025
16 checks passed
@raysan5
Copy link
Owner

raysan5 commented Oct 15, 2025

@JohnnyCena123 thanks for the addition. Did you verify it generates same results as the alternative implementation by @manuel5975p, #5266?

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 this pull request may close these issues.

4 participants