[PATCH 0/3] ASoC: SOF: debug: cleanups

[PATCH 0/3] ASoC: SOF: debug: cleanups

[Pierre-Louis Bossart]

cleanups of allocation and error handling

Guennadi Liakhovetski (3):
  ASoC: SOF: fix debugfs initialisation error handling
  ASoC: SOF: only allocate debugfs cache buffers for IPC flood entries
  ASoC: SOF: remove superfluous NULL check in debugfs read

 sound/soc/sof/core.c  |  5 +++--
 sound/soc/sof/debug.c | 21 +++++++++++----------
 2 files changed, 14 insertions(+), 12 deletions(-)

-- 
2.25.1

[PATCH 1/3] ASoC: SOF: fix debugfs initialisation error handling

[Pierre-Louis Bossart]

From: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

If debugfs initialisation fails partially in sof_probe_continue() some
debugfs files and the root directory might have been created
successfully. They have to be cleaned up if some of them failed too.

Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Xiuli Pan <xiulipan@outlook.com>
Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c
index 6d8f7d9fd192..495295a34c3a 100644
--- a/sound/soc/sof/core.c
+++ b/sound/soc/sof/core.c
@@ -154,7 +154,7 @@ static int sof_probe_continue(struct snd_sof_dev *sdev)
 	if (ret < 0) {
 		dev_err(sdev->dev, "error: failed to get machine info %d\n",
 			ret);
-		goto dbg_err;
+		goto dsp_err;
 	}
 
 	/* set up platform component driver */
@@ -257,8 +257,9 @@ static int sof_probe_continue(struct snd_sof_dev *sdev)
 fw_load_err:
 	snd_sof_ipc_free(sdev);
 ipc_err:
-	snd_sof_free_debug(sdev);
 dbg_err:
+	snd_sof_free_debug(sdev);
+dsp_err:
 	snd_sof_remove(sdev);
 
 	/* all resources freed, update state to match */
-- 
2.25.1

[PATCH 2/3] ASoC: SOF: only allocate debugfs cache buffers for IPC flood entries

[Pierre-Louis Bossart]

From: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

snd_sof_debugfs_buf_item() is an exported function and is called from
different locations to initialise different debugfs entries. However
.cache_buf is only needed for IPC flood entries. Limit allocations
respectively.

Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Xiuli Pan <xiulipan@outlook.com>
Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/debug.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/sound/soc/sof/debug.c b/sound/soc/sof/debug.c
index 715a374b33cf..778c7d028493 100644
--- a/sound/soc/sof/debug.c
+++ b/sound/soc/sof/debug.c
@@ -609,14 +609,16 @@ int snd_sof_debugfs_buf_item(struct snd_sof_dev *sdev,
 	dfse->sdev = sdev;
 
 #if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST)
-	/*
-	 * cache_buf is unused for SOF_DFSENTRY_TYPE_BUF debugfs entries.
-	 * So, use it to save the results of the last IPC flood test.
-	 */
-	dfse->cache_buf = devm_kzalloc(sdev->dev, IPC_FLOOD_TEST_RESULT_LEN,
-				       GFP_KERNEL);
-	if (!dfse->cache_buf)
-		return -ENOMEM;
+	if (!strncmp(name, "ipc_flood", strlen("ipc_flood"))) {
+		/*
+		 * cache_buf is unused for SOF_DFSENTRY_TYPE_BUF debugfs entries.
+		 * So, use it to save the results of the last IPC flood test.
+		 */
+		dfse->cache_buf = devm_kzalloc(sdev->dev, IPC_FLOOD_TEST_RESULT_LEN,
+					       GFP_KERNEL);
+		if (!dfse->cache_buf)
+			return -ENOMEM;
+	}
 #endif
 
 	debugfs_create_file(name, mode, sdev->debugfs_root, dfse,
-- 
2.25.1

[PATCH 3/3] ASoC: SOF: remove superfluous NULL check in debugfs read

[Pierre-Louis Bossart]

From: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

When reading from IPC flood debugfs entries no need to check whether
.cache_buf is NULL - it's impossible since otherwise the initialisation
would have failed. This also fixes a klocwork reported issue:

passed to function and may be dereferenced there by passing argument 2
to function 'memcpy' at line 510.
sound/soc/sof/debug.c:510 | sof_dfsentry_read()

Reported-by: Keqiao Zhang <keqiao.zhang@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Xiuli Pan <xiulipan@outlook.com>
Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/debug.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sound/soc/sof/debug.c b/sound/soc/sof/debug.c
index 778c7d028493..a51a928ea40a 100644
--- a/sound/soc/sof/debug.c
+++ b/sound/soc/sof/debug.c
@@ -451,8 +451,7 @@ static ssize_t sof_dfsentry_read(struct file *file, char __user *buffer,
 
 	dentry = file->f_path.dentry;
 	if ((!strcmp(dentry->d_name.name, "ipc_flood_count") ||
-	     !strcmp(dentry->d_name.name, "ipc_flood_duration_ms")) &&
-	    dfse->cache_buf) {
+	     !strcmp(dentry->d_name.name, "ipc_flood_duration_ms"))) {
 		if (*ppos)
 			return 0;
 
-- 
2.25.1

Re: [PATCH 0/3] ASoC: SOF: debug: cleanups

[Mark Brown]

On Mon, 15 Mar 2021 11:39:29 -0500, Pierre-Louis Bossart wrote:
> cleanups of allocation and error handling
> 
> Guennadi Liakhovetski (3):
>   ASoC: SOF: fix debugfs initialisation error handling
>   ASoC: SOF: only allocate debugfs cache buffers for IPC flood entries
>   ASoC: SOF: remove superfluous NULL check in debugfs read
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/3] ASoC: SOF: fix debugfs initialisation error handling
      commit: 11fd6d8e097b5b179ea445e0206aaefc47e62845
[2/3] ASoC: SOF: only allocate debugfs cache buffers for IPC flood entries
      commit: 72c35856b5edc3f734be5699e9f6737190a1d897
[3/3] ASoC: SOF: remove superfluous NULL check in debugfs read
      commit: 97f53046d746bef513d5fbaac53eedb011968407

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark