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.

Sign up for GitHub

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

Jump to bottom

remoteproc: fix memory issue if load resource table failed #362

Merged
merged 1 commit into from
Mar 30, 2022
Merged

remoteproc: fix memory issue if load resource table failed #362

merged 1 commit into from
Mar 30, 2022

Conversation

anchao
Copy link
Contributor

@anchao anchao commented Mar 19, 2022

remoteproc: fix memory issue if load resource table failed

@arnopo arnopo assigned edmooring and arnopo and unassigned edmooring and arnopo Mar 21, 2022
@arnopo arnopo requested review from arnopo and edmooring March 21, 2022 16:36
@arnopo
Copy link
Collaborator

arnopo commented Mar 21, 2022

I can not figure out if this patch fixes something or is just a clean-up...
Please could you add a commit content message explaining the issue your faced? This will also resolve compliance check complain.
If it is just a clean-up , please also update the commit subject.

@edmooring
Copy link
Contributor

I'm not sure I understand what this is supposed to do. The diff looks like it fixes an error in the way error returns are done in remoteproc_get_rsc_table(), but the actual code shows similar changes that were made 4 years ago: https://github.com/OpenAMP/open-amp/blame/b139a1ff11d16c85f1b04a852b0faa7fafa8fe69/lib/remoteproc/remoteproc.c#L123

@xiaoxiang781216
Copy link
Collaborator

xiaoxiang781216 commented Mar 22, 2022 •
edited
Loading

The old code pass the error code to metal_free_memory in failure which is obviously wrong:

  1. Generate the memory leak
  2. May corrupt the heap manager

@anchao please update the commit message to pass the CI.

@anchao anchao force-pushed the 22031901 branch 2 times, most recently from fac2230 to 5fd7ce1 Compare March 22, 2022 03:40
@anchao
remoteproc: fix memory issue if load resource table failed
2a42933 
The old code pass the error code to metal_free_memory in failure
which is obviously wrong:

1. Generate the memory leak
2. May corrupt the heap manager

Signed-off-by: chao an <anchao@xiaomi.com>
arnopo
arnopo approved these changes Mar 22, 2022
View reviewed changes
Copy link
Collaborator

@arnopo arnopo left a comment

Choose a reason for hiding this comment

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

Clear for me now, Thanks!

@arnopo arnopo added this to the Release V2022.04 milestone Mar 22, 2022
edmooring
edmooring approved these changes Mar 24, 2022
View reviewed changes
Copy link
Contributor

@edmooring edmooring left a comment

Choose a reason for hiding this comment

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

This looks good to me.

@arnopo arnopo merged commit 0307c3f into OpenAMP:main Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arnopo arnopo arnopo approved these changes

@edmooring edmooring edmooring approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Release V2022.04
Development

Successfully merging this pull request may close these issues.
4 participants
@anchao @arnopo @edmooring @xiaoxiang781216