Skip to content

Switch mlk_polyvec and mlk_polymat to struct wrappers #1115

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Switch mlk_polyvec and mlk_polymat to struct wrappers #1115

wants to merge 1 commit into from

Conversation

hanno-becker
Copy link
Contributor

  • Change mlk_polyvec back to struct { mlk_poly vec[MLKEM_K]; }
  • Change mlk_polymat to struct { mlk_polyvec vec[MLKEM_K]; }
  • Update all function signatures to use pointer style
  • Fix all implementations to use struct member access
  • Update tests, benchmarks, and CBMC harnesses
  • Add consistent const annotations

@hanno-becker
Switch mlk_polyvec and mlk_polymat to struct wrappers
d83012d 
- Change mlk_polyvec back to struct { mlk_poly vec[MLKEM_K]; }
- Change mlk_polymat to struct { mlk_polyvec vec[MLKEM_K]; }
- Update all function signatures to use pointer style
- Fix all implementations to use struct member access
- Update tests, benchmarks, and CBMC harnesses
- Add consistent const annotations

Signed-off-by: Hanno Becker <beckphan@amazon.co.uk>
@hanno-becker hanno-becker requested a review from a team as a code owner July 9, 2025 19:49
@hanno-becker hanno-becker mentioned this pull request Jul 9, 2025
Draft
mkannwischer
mkannwischer reviewed Jul 10, 2025
View reviewed changes
Copy link
Contributor

@mkannwischer mkannwischer left a comment

Choose a reason for hiding this comment

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

Could you please elaborate why you would like to make this change?

@hanno-becker
Copy link
Contributor Author

hanno-becker commented Jul 10, 2025 •
edited
Loading

@rod-chapman suggested that this unblocks #906. Adjusting it to check if that's true.

@mkannwischer
Copy link
Contributor

@rod-chapman suggested that this unblocks #906. Adjusting it to check if that's true.

Should this be converted to a draft PR in that case?

@hanno-becker
Copy link
Contributor Author

@mkannwischer We could. One could make the argument, though, that the only reason to introduce mlk_polymat/vec as a array was to fix the UB in gen_matrix(), which is now moot given @rod-chapman's #1112.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkannwischer mkannwischer mkannwischer left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hanno-becker @mkannwischer