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 nullptr deref in AudioFileProcessor (qt6 branch) #7532

Merged
merged 5 commits into from
Oct 6, 2024

Conversation

RiedleroD
Copy link
Contributor

opening AFP was consistently crashing LMMS on the qt6 branch before - this patch checks whether the passed event pointer is a nullptr, and only derefs it if not.

@RiedleroD RiedleroD changed the title fix nullptr deref in AudioFileProcessor fix nullptr deref in AudioFileProcessor (qt6 branch) Oct 3, 2024
@RiedleroD
Copy link
Contributor Author

it seems like the docs on running tests are outdated - took me a bit to figure those out.

In any case, everything looks fine to me.

@RiedleroD RiedleroD marked this pull request as ready for review October 3, 2024 19:31
@Rossmaxx
Copy link
Contributor

Rossmaxx commented Oct 4, 2024

Btw saw it just now that this is against qt6. Any chance this might go on master too?

@RiedleroD
Copy link
Contributor Author

RiedleroD commented Oct 4, 2024

Any chance this might go on master too?

Only once qt6 support gets merged. The bug it fixes only occurs because of the position() function that had to be made for qt6 support. Master checks for nullptr correctly for what it's trying to do.

extract check to pointerCloseToStartEndOrLoop()
Copy link
Contributor

@michaelgregorius michaelgregorius left a comment

Choose a reason for hiding this comment

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

Some last remarks but other than that it looks good to me now.

@michaelgregorius michaelgregorius merged commit f1c852b into LMMS:qt6 Oct 6, 2024
10 checks passed
@RiedleroD RiedleroD deleted the qt6-fix-afp-nullptr-deref branch October 6, 2024 17:29
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.

3 participants