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(client): Update rnnoise-src submodule #4850

Merged
merged 1 commit into from Mar 11, 2021

Conversation

davidebeatrici
Copy link
Member

This fixes a heap corruption that was originally discovered by @PaulDana on #mumble-dev.

In addition to that, all upstream changes are integrated.

Copy link
Member

@Krzmbrzl Krzmbrzl left a comment

Choose a reason for hiding this comment

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

This is not a FEAT.
If it only were the submodule update I would call this MAINT, but given that it fixes an issue it should probably be a FIX.

@Kissaki
Copy link
Member

Kissaki commented Mar 11, 2021

Can we split these two concerns into two separate commits?

@davidebeatrici
Copy link
Member Author

What do you mean?

@Kissaki
Copy link
Member

Kissaki commented Mar 11, 2021

Isn’t this two completely separate concerns?

  • Updating the submodule, with I don't know what changes (MAINT)
  • Implementing a heap corruption bugfix (FIX)

@davidebeatrici
Copy link
Member Author

No, the heap corruption fix is part of the submodule.

@davidebeatrici davidebeatrici changed the title FEAT(client): Update rnnoise-src submodule FIX(client): Update rnnoise-src submodule Mar 11, 2021
@Krzmbrzl
Copy link
Member

@davidebeatrici the commit message should reference the fixed issue.

@davidebeatrici
Copy link
Member Author

I prefer not to do that because in case of force-pushes the related pull request gets spammed.

@Krzmbrzl
Copy link
Member

That doesn't cost us anything though. But by referencing the commit with the issue we get a linked history which is very valuable when someone has to look back at it ☝️

Besides: There are no other changes needed here and thus there is no force pushing expected anymore.

This fixes a heap corruption that was originally discovered by PaulDana on #mumble-dev.

In addition to that, all upstream changes are integrated.

Fixes mumble-voip#4842.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Debug build immediately crashing with HEAP CORRUPTION error
3 participants