Skip to content

Free a couple of memory leaks in the SOF plugin - #9055

Merged
kv2019i merged 1 commit into
thesofproject:mainfrom
ranj063:main-041624
Jun 19, 2024
Merged

Free a couple of memory leaks in the SOF plugin#9055
kv2019i merged 1 commit into
thesofproject:mainfrom
ranj063:main-041624

Conversation

@ranj063

@ranj063ranj063 commented Apr 16, 2024

Copy link
Copy Markdown
Collaborator

No description provided.

kv2019i
kv2019i reviewed Apr 17, 2024
View reviewed changes

@kv2019ikv2019i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One comment to first patch, otherwise looks good.

Comment threadtools/plugin/alsaplug/tplg.c Outdated
if (available_fmts->output_pin_fmts)
free(available_fmts->output_pin_fmts);
if (available_fmts->input_pin_fmts)
free(available_fmts->input_pin_fmts);

@kv2019ikv2019iApr 17, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Isn't this if-check redundannt, NULL is ok to free

@ranj063ranj063Apr 17, 2024

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

ack fixed now

@kv2019ikv2019iApr 18, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@ranj063 the fix went into the wrong patch

lgirdwood
lgirdwood reviewed Apr 17, 2024
View reviewed changes

@lgirdwoodlgirdwood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ack, LGTM with comment from @kv2019i.
Btw, does this mean valgrind runs with 0 errors ?

@ranj063

ranj063 commented Apr 17, 2024

Copy link
Copy Markdown
CollaboratorAuthor

Ack, LGTM with comment from @kv2019i. Btw, does this mean valgrind runs with 0 errors ?

not yet @lgirdwood. Im working to get there

lgirdwood
lgirdwood reviewed Apr 18, 2024
View reviewed changes

@lgirdwoodlgirdwood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should these 2 patches be squashed ?

@lgirdwood

lgirdwood commented May 14, 2024

Copy link
Copy Markdown
Member

@ranj063 ping.

@lgirdwoodlgirdwood added this to the v2.10 milestone Jun 11, 2024
@lgirdwood

lgirdwood commented Jun 11, 2024

Copy link
Copy Markdown
Member

@ranj063 ping

@ranj063
tools: plugin: tplg: Fix a couple of memory leaks
60b1c5f
Free the memory allocated for tplg base and available input/output
formats when the topology is freed.
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
@ranj063

ranj063 commented Jun 13, 2024

Copy link
Copy Markdown
CollaboratorAuthor

@ranj063 ping

Sorry for the delay @lgirdwood . Fixed now

@kv2019i

kv2019i commented Jun 17, 2024

Copy link
Copy Markdown
Collaborator

@wszypelt Can you check? This seems to be stuck..

@kv2019i

kv2019i commented Jun 19, 2024

Copy link
Copy Markdown
Collaborator

@wszypelt@ranj063 Still seeing failure. Shall I just proceed with merge? This is touching plugin code that is not even built into the FW, so the quickbuild tests are in no way relevant to this change.

@wszypelt

wszypelt commented Jun 19, 2024•
edited
Loading

Copy link
Copy Markdown

@kv2019i after checking the tests, one issue appeared, which is sporadic and it has nothing to do with PR, in my opinion you can merge, but if you still have about an hour, the result should already be green

@kv2019i
kv2019i merged commit 9b7fffd into thesofproject:mainJun 19, 2024
singalsu added a commit to singalsu/sof that referenced this pull request Aug 29, 2024
@singalsu
Copy Ranjani's plugin leak fixes fromthesofproject#9055.
389a9e0
Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lgirdwoodlgirdwoodlgirdwood left review comments
@abonislawskiabonislawskiabonislawski approved these changes
@kv2019ikv2019ikv2019i approved these changes
@plbossartplbossartAwaiting requested review from plbossartplbossart is a code owner
@mmaka1mmaka1Awaiting requested review from mmaka1mmaka1 is a code owner
@lbetlejlbetlejAwaiting requested review from lbetlejlbetlej is a code owner
@dbalutadbalutaAwaiting requested review from dbalutadbaluta is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v2.10

Development

Successfully merging this pull request may close these issues.

5 participants

@ranj063@lgirdwood@kv2019i@wszypelt@abonislawski