testbench: improvements for valgrind and debug #4408
Conversation
lgirdwood
requested review from
dbaluta,
lbetlej,
mmaka1,
plbossart and
ranj063
as code owners
Buffer and params not initialised to zero like the other IPC structures used by testbench. Fix this. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Lets be more helpful to the user. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Move the comp free to after the results and free the pipeline. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Testbench crashed when a in/out file has not . extension. Fix this. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
All the user to specify the number of copies. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
lgirdwood
force-pushed
the
lrg/topic/testbench2
branch
from
d7bff6b to
65c2b3e
Compare
| { | ||
| struct sof *sof = (struct sof *)dev; | ||
| struct sof_ipc_buffer buffer; | ||
| struct sof_ipc_buffer buffer = {0}; |
There was a problem hiding this comment.
Isn't the job of tplg_load_buffer() to initialize buffer? What error does this silence?
There was a problem hiding this comment.
testbench does not use all the data members as some are specific to HW config. This was only missing from the two "IPCs" in this PR.
There was a problem hiding this comment.
I don't understand sorry, I don't have the full picture. What was the error reported and in which function?
There was a problem hiding this comment.
Use of uninitialized data in FW buffer logic due to testbench not initializing it. Not an issue with kernel as it clears. Testbench bug.
There was a problem hiding this comment.
I understood that the error was testbench specific, my concern is that this change hides any "real" initialization bug in tplg_load_buffer in the future.
There was a problem hiding this comment.
Got you now, yep the userspace tplg parser is not setting any variables if it cant find tokens for them. I will fix this as another PR as it means removing more testbench code here and also testing other tools that used the shared tplg parser.
| struct ipc_comp_dev *pcm_dev; | ||
| struct comp_dev *cd; | ||
| struct sof_ipc_pcm_params params; | ||
| struct sof_ipc_pcm_params params = {0}; |
There was a problem hiding this comment.
This looks like the initialization of some field(s) is missing below and this is merely hiding the issue.
'const' documents APIs and catches bugs. This came while discussing thesofproject#4408 Signed-off-by: Marc Herbert <marc.herbert@intel.com>
'const' documents APIs and catches bugs. This came while discussing #4408 Signed-off-by: Marc Herbert <marc.herbert@intel.com>
'const' documents APIs and catches bugs. This came while discussing thesofproject#4408 Signed-off-by: Marc Herbert <marc.herbert@intel.com>
'const' documents APIs and catches bugs. This came while discussing #4408 Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Add some improvements to support valgrind and eliminate all valgrind errors. Also allow users to specify number of copies.