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

Fix: CRC32 type error #1556

Merged
merged 6 commits into from
Jul 22, 2023
Merged

Fix: CRC32 type error #1556

merged 6 commits into from
Jul 22, 2023

Conversation

dragonmux
Copy link
Member

Detailed description

While toying with LTO and doing a BMDA build, GCC warned of a type error in the CRC32 header. On checking it out, the type for the length parameter was indeed mismatched between header and both implementations.

This PR addresses this and does cleanup on the CRC32 implementations to bring them up to standard and make them easier to read and understand. Tested against a LPC4370 on both BMP and using BMDA, this offers no functional change but does fix some undefined behaviour.

Your checklist for this pull request

  • I've read the Code of Conduct
  • I've read the guidelines for contributing to this repository
  • It builds for hardware native (make PROBE_HOST=native)
  • It builds as BMDA (make PROBE_HOST=hosted)
  • I've tested it to the best of my ability
  • My commit messages provide a useful short description of what the commits do

Closing issues

@dragonmux dragonmux added the Bug Confirmed bug label Jul 22, 2023
@dragonmux dragonmux added this to the v1.10 milestone Jul 22, 2023
@dragonmux dragonmux requested a review from esden July 22, 2023 14:10
Copy link
Member

@esden esden left a comment

Choose a reason for hiding this comment

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

LGTM

@esden esden merged commit 6e397a5 into main Jul 22, 2023
6 checks passed
@dragonmux dragonmux deleted the fix/crc32-type-error branch July 22, 2023 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Confirmed bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants