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

Revert "module_adapter: avoid module init crash in case of ipc data invalid" #8303

Merged
merged 1 commit into from
Oct 13, 2023

Conversation

iuliana-prodan
Copy link
Contributor

@iuliana-prodan iuliana-prodan commented Oct 10, 2023

Revert "module_adapter: avoid module init crash in case of ipc data invalid"

This reverts 'commit e847c8b ("module_adapter: avoid module
init crash in case of ipc data invalid")'.

No data is not an invalid case for mixer, for example.

Fixes: #8265
Signed-off-by: Iuliana Prodan [email protected]

@@ -79,6 +79,9 @@ int module_adapter_init_data(struct comp_dev *dev,
return ret;
}
dst->init_data = dst->data;
} else if (size == 0) {
/* valid case for mixer */
return 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

if this is the case, do we still need keep line 85 - 87?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@btian1 true, size can't be negative. So maybe we just do a full revert.

It does open questions how to address the original commit e847c8b . Do we need a SRC specific patch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree @kv2019i , I'll do a full revert of this e847c8b, but the revert doesn't apply cleanly now because of the ipc3/ipc4 split.

I've kept the check for negative size to not break something on Intel targets - even though the explanation for adding that commit is limited.

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.

I'd suggest a revert and then do a follow-up to make a new fix to avoiding module init crashes (if a generic module level solution is possible, not sure it is).

@@ -79,6 +79,9 @@ int module_adapter_init_data(struct comp_dev *dev,
return ret;
}
dst->init_data = dst->data;
} else if (size == 0) {
/* valid case for mixer */
return 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@btian1 true, size can't be negative. So maybe we just do a full revert.

It does open questions how to address the original commit e847c8b . Do we need a SRC specific patch?

…nvalid"

This reverts 'commit e847c8b ("module_adapter: avoid module
init crash in case of ipc data invalid")'.

No data is not an invalid case for mixer, for example.

Fixes: thesofproject#8265
Signed-off-by: Iuliana Prodan <[email protected]>
@iuliana-prodan iuliana-prodan changed the title ipc3: module adapter: do not crash in case of no data Revert "module_adapter: avoid module init crash in case of ipc data invalid" Oct 12, 2023
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.

Looks good. @btian1 can you do a follow-up?

@btian1
Copy link
Contributor

btian1 commented Oct 13, 2023

an you do a follow-up?

Looks good. @btian1 can you do a follow-up?

e847c8b, for me this is a safeguard to avoid module init crash, I remember there is another patch, I forgot the commit name to fix similar issue.
let's merge this first, then check if have any further issues.

@dbaluta dbaluta merged commit bb8d6ba into thesofproject:main Oct 13, 2023
38 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.

[BUG] Unable to load tplg with mixer or DRC component
4 participants