Skip to content

Conversation

@jsarha
Copy link
Contributor

@jsarha jsarha commented Sep 4, 2025

Assign mod->stream_params to NULL aftere it is freed. If this is not done comp_free() tries to free it again couple of lines later, when it calls module_adapter_free(). Removing test_free(mod->stream_params) does not fix the issue because cmocka framework provided test_alloc() and test_free() are not equivalent to malloc() and free().

Copilot AI review requested due to automatic review settings September 4, 2025 19:44
Assign mod->stream_params to NULL aftere it is freed. If this is not
done comp_free() tries to free it again couple of lines later, when it
calls module_adapter_free(). Removing test_free(mod->stream_params)
does not fix the issue because cmocka framework provided test_alloc()
and test_free() are not equivalent to malloc() and free().

Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a test crash in cmocka tests for eq_iir and eq_fir modules by preventing double-free errors. The issue occurs when comp_free() attempts to free mod->stream_params after it has already been freed in the teardown function.

  • Set mod->stream_params to NULL after freeing it in teardown functions
  • Prevent double-free crashes when comp_free() calls module_adapter_free()

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
test/cmocka/src/audio/eq_iir/eq_iir_process.c Adds NULL assignment after freeing stream_params to prevent double-free
test/cmocka/src/audio/eq_fir/eq_fir_process.c Adds NULL assignment after freeing stream_params to prevent double-free

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

Good catch @jsarha !

@kv2019i kv2019i merged commit 1391978 into thesofproject:main Sep 5, 2025
39 of 45 checks passed
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.

4 participants