smart_amp: fix return errors#9387
Merged
lgirdwood merged 1 commit intothesofproject:mainfrom Aug 23, 2024
Merged
Conversation
johnylin76
reviewed
Aug 22, 2024
src/audio/smart_amp/smart_amp.c
Outdated
| if (ret <= 0) | ||
| if (ret <= 0) { | ||
| if (ret == 0) | ||
| ret = -EINVAL; |
Contributor
There was a problem hiding this comment.
I'll propose to return early without error
if (ret < 0)
goto error;
/* no memory requirement */
if (ret == 0)
return 0;
/* allocate the memory block when returned size > 0. */
...
Member
There was a problem hiding this comment.
We should also call this out in an inline comment that 0 here is an error code from smart amp (otherwise people will try and fix)
Contributor
Author
There was a problem hiding this comment.
I think what @johnylin76 is saying is that its not an error and was incorrectly treated as it if it was an error. I believe 0 is "not needed" and shouldnt have been triggering the error path.
cujomalainey
commented
Aug 22, 2024
| @@ -111,8 +111,11 @@ static ssize_t smart_amp_alloc_mod_memblk(struct smart_amp_data *sad, | |||
|
|
|||
| /* query the required size from inner model. */ | |||
| ret = mod->mod_ops->query_memblk_size(mod, blk); | |||
Contributor
Author
There was a problem hiding this comment.
@johnylin76 FYI we are exploring 64bit builds now so sizes might differ between builds. It might be wiser to have the API use ssize_t rather than int. Minor nit for you to consider.
If 0 is a error then override it so the outer function doesn't think we ran successfully and then hit a null ptr after we freed everything. Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
501790b to
6c6dea1
Compare
lyakh
approved these changes
Aug 23, 2024
lgirdwood
approved these changes
Aug 23, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If 0 is a error then override it so the outer function doesn't think we ran successfully and then hit a null ptr after we freed everything.