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

Uninitialized memory access in MusicDSPModel constructor #12

Open
L3337 opened this issue Jan 22, 2023 · 0 comments
Open

Uninitialized memory access in MusicDSPModel constructor #12

L3337 opened this issue Jan 22, 2023 · 0 comments

Comments

@L3337
Copy link

L3337 commented Jan 22, 2023

Nice project.

I spotted this while I was studying the code:

The constructor calls SetCutoff
SetCutoff calls SetResonance

2 problems AFAICT with that:

  • resonance is uninitialized on the first call, could conceivably crash if arbitrary junk at that memory address was a NaN or similar
  • SetCutoff calling SetResonance with the calculated resonance coefficient and not a 0.0 to 1.0 resonance value is probably(?) incorrect behavior. With DSP, you never really know, but in this case it results in different filter coefficients depending on whether SetResonance was called before SetCutoff. If there is a hard requirement for the order the methods are called in, the methods should just be merged into a single method, especially since one already calls the other.

Would also recommend to create a test suite to catch memory errors automatically. Write a separate executable that loads some arbitrary wav file, runs all of the filter permutations against it, then run the executable through valgrind to check for memory errors.

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

No branches or pull requests

1 participant