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

ParamOption::SetValue potential crash #91

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jorisdejong
Copy link
Contributor

The orginal function checks if the value is in range, but then still set the actual option using the out of range value

@MennoVink
Copy link
Collaborator

@flyingrub another quickstart param thing for you :)
I wonder if it should go to zero though, shouldn't it clamp rather than just choose 0? ie clamp the value to be between [0..options.size())

@jorisdejong
Copy link
Contributor Author

@flyingrub The set to zero is unchanged from your original code. I'd prefer clamping to options.size() too, but I assumed it was resetting to 0 for a reason :)

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