Skip to content

fix: add scalar conversion checks in a few critical places#1069

Merged
aviator-app[bot] merged 1 commit intomainfrom
fix/scalar-conversion-checks-in-manual-code
Jan 2, 2024
Merged

fix: add scalar conversion checks in a few critical places#1069
aviator-app[bot] merged 1 commit intomainfrom
fix/scalar-conversion-checks-in-manual-code

Conversation

@szhorvat
Copy link
Copy Markdown
Member

@szhorvat szhorvat commented Jan 2, 2024

This PR adds checks for converting scalars (especially scalar integers) to C in a few heavily used and critical places. This is similar to #1051 but for manually written code. I will not add the checks for all manually written code, only for the most important parts. The rest should be handled by moving functions to auto-generation.

I needed to change a test which verified that make_graph(NULL, n = NULL) worked. IMO using n=NULL should not be allowed an in fact this probably triggered an invalid memory access, as the code would index into a zero-length vector (which is somehow not caught by the sanitizer). I am not sure why this test was present.

Because of the presence of this unusual test, running revdepchecks would be a good idea.

@aviator-app
Copy link
Copy Markdown
Contributor

aviator-app bot commented Jan 2, 2024

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes.
Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This PR was merged using Aviator.


See the real-time status of this PR on the Aviator webapp.

Use the Aviator Chrome Extension to see the status of your PR within GitHub.

@szhorvat szhorvat requested a review from krlmlr January 2, 2024 14:09
@szhorvat
Copy link
Copy Markdown
Member Author

szhorvat commented Jan 2, 2024

The test was added in 6453014

#1068 effectively reverts the rest of that commit.

@krlmlr
Copy link
Copy Markdown
Contributor

krlmlr commented Jan 2, 2024

Thanks!

@aviator-app aviator-app bot force-pushed the fix/scalar-conversion-checks-in-manual-code branch from 6bf98a8 to bf3a283 Compare January 2, 2024 19:52
@aviator-app aviator-app bot merged commit 87813a2 into main Jan 2, 2024
@aviator-app aviator-app bot deleted the fix/scalar-conversion-checks-in-manual-code branch January 2, 2024 20:20
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 2, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants