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

gh-111178: fix UBSan failures in Modules/{blake2,md5,sha1,sha2,sha3}module.c #128248

Merged
merged 14 commits into from
Jan 27, 2025

Conversation

picnixz
Copy link
Member

@picnixz picnixz commented Dec 25, 2024

Effective commits

  • fix UBSan failures in blake2module.c
  • fix UBSan failures in md5module.c
  • fix UBSan failures in sha1module.c
  • fix UBSan failures in sha2module.c
  • fix UBSan failures in sha3module.c

Cosmetics

  • suppress unused return values
  • remove redundant casts in constructors
  • suppress unused parameters in {md5,sha*,blake2}module.c

@picnixz picnixz changed the title gh-111178: fix UBSan failures in Modules/{sha1,sha2,sh3,md5}module.c gh-111178: fix UBSan failures in Modules/{blake2,md5,sha1,sha2,sha3}module.c Dec 25, 2024
@encukou encukou merged commit 922cfec into python:main Jan 27, 2025
40 checks passed
@encukou
Copy link
Member

encukou commented Jan 27, 2025

One nitpick to keep in mind for the future: C standard reserves identifiers that start with underscore and capital letter, so e.g. defining _Blake2Object_CAST is technically undefined behaviour.
(Same with _Py*, of course. It'd be nicer to not add new ones in addition to _Py.)

@picnixz picnixz deleted the fix/ubsan/sha-111178 branch February 2, 2025 21:52
@picnixz
Copy link
Member Author

picnixz commented Feb 2, 2025

Oh... I was not aware of this. All my branches use an underscore so that the macro is really thought as being private. I can update the PRs locally as they are not yet pushed if you think it's something we should do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants