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: prevent potential crash in handleBezier and in refreshRate due to invalid input #6246

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tachyglossues
Copy link

Describe your PR, what does it fix/add?

before, when the conf file contained anything other than a float in the 4 bezier curve generation points for annimations, it crashed Hyprland, which now returns an error message.

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

This is my first PR, so please check that I haven't done anything wrong.

Is it ready for merging, or does it need work?

It's ready

before, when the conf file contained anything other than a float in the 4 bezier curve generation points for annimations, it crashed Hyprland, which now returns an error message.
@vaxerski vaxerski force-pushed the main branch 2 times, most recently from 358e59e to 3fd6c1b Compare June 3, 2024 16:46
@Micovec
Copy link
Contributor

Micovec commented Jun 4, 2024

Related #6249 I think that a util function should be created instead (as suggested by Vaxry in that issue).

@tachyglossues
Copy link
Author

Related #6249 I think that a util function should be created instead (as suggested by Vaxry in that issue).

In this PR I've just fixed #6249 and I don't know if it's better to create a function. Because a simple try/catch is enough to solve this type of problem. And from the few tests I've done I haven't found the same problem elsewhere.

@tachyglossues tachyglossues changed the title fix: prevent potential crash in handleBezier due to invalid input fix: prevent potential crash in handleBezier and in refreshRate due to invalid input Jun 4, 2024
@Micovec
Copy link
Contributor

Micovec commented Jun 4, 2024

I don't want to be rude, but there are many more places with the same problem. Just look two lines up from your first change.

Also I'm not a reviewer but you don't want to do:

...
} catch (...) {
    // An oopsie happened
}

It catches everything including things like std::bad_alloc. And also you seem to forgot about std::out_of_range.

@bakicelebi
Copy link

@tachyglossues are you going to continue to work on this? This issue is annoying the hell out of me with vscode autosave. If you are not gonna continue working on this I will raise my own PR.

@tachyglossues
Copy link
Author

@tachyglossues are you going to continue to work on this? This issue is annoying the hell out of me with vscode autosave. If you are not gonna continue working on this I will raise my own PR.

No, because the arguments are handled differently in the code, and I don't have the necessary C++ skills to refactor the code without making a disaster of it.

Copy link

@vignesh1507 vignesh1507 left a comment

Choose a reason for hiding this comment

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

You can add few type hints to your updated code snippet.

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.

5 participants