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

Make safe the functions of the "fit" module #145

Merged
merged 2 commits into from
Mar 23, 2024

Conversation

Chris00
Copy link
Collaborator

@Chris00 Chris00 commented Mar 21, 2024

The functions fit::linear,... were not checking that the length n was not too large (in which case the function would access memory outside the slice 😨). This PR fixes that.

P.S. IMHO, the strides and length parameters should be optional. For example a trait for “slice or (slice, stride)” would be simple and convenient (for strides, not the length n). For a better “Rust like” user experience, bigger changes are needed (I can try to propose something if you agree that such a change is beneficial).

@GuillaumeGomez
Copy link
Owner

Thanks for the added checks! Please fix the CI errors then I'll merge.

@Chris00
Copy link
Collaborator Author

Chris00 commented Mar 23, 2024

I fixed the cargo fmt part. The other error is unrelated to my changes — and would require a redesign of the cblas interface as, say, idamax returning an Index that no function accepts as argument makes idamax useless.

@GuillaumeGomez
Copy link
Owner

Indeed! New rust unused items analyzer is great. I'll send a fix for it. Thanks!

@GuillaumeGomez GuillaumeGomez merged commit 718ecee into GuillaumeGomez:master Mar 23, 2024
4 of 6 checks passed
@Chris00 Chris00 mentioned this pull request Mar 23, 2024
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.

2 participants