Re: [alsa-devel] [PATCH v2 114/146] ASoC: soc-topology: use modern dai_link style

On 06 Jun 2019 13:19:14 +0900 Kuninori Morimoto kuninori.morimoto.gx@renesas.com wrote:

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com

ASoC is now supporting modern style dai_link (= snd_soc_dai_link_component) for CPU/Codec/Platform. This patch switches to use it.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com

sound/soc/soc-topology.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index 3299ebb..f485f7f 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -560,7 +560,7 @@ static void remove_link(struct snd_soc_component *comp, kfree(link->name); kfree(link->stream_name);

• kfree(link->cpu_dai_name);

• kfree(link->cpus->dai_name);

  list_del(&dobj->list); snd_soc_remove_dai_link(comp->card, link);

@@ -1879,12 +1879,22 @@ static int soc_tplg_fe_link_create(struct soc_tplg *tplg, struct snd_soc_tplg_pcm *pcm) { struct snd_soc_dai_link *link;

• struct snd_soc_dai_link_component *dlc; int ret;

• link = kzalloc(sizeof(struct snd_soc_dai_link), GFP_KERNEL);

• /* link + cpu + codec */
• link = kzalloc(sizeof(*link) + (2 * sizeof(*dlc)),

GFP_KERNEL); if (link == NULL) return -ENOMEM;

• dlc = (struct snd_soc_dai_link_component *)(link + 1);
• link->cpus = &dlc[0];
• link->codecs = &dlc[1];

While I understand what is going on here, I find this bit ugly. Can it perhaps be changed to something like:

link = kzalloc(sizeof(*link), GFP_KERNEL); if (link == NULL) return -ENOMEM; link->cpus = kzalloc(sizeof(*dlc), GFP_KERNEL); if (link->cpus == NULL) { ret = -ENOMEM; goto err; } link->codecs = kzalloc(sizeof(*dlc), GFP_KERNEL); if (link->cpus == NULL) { ret = -ENOMEM; goto err; }

(...) err: kfree(link->cpus); kfree(link); return ret;

While it has a bit more of boiler plate it's easier to follow.

And while I look at soc_tplg_fe_link_create() it could use some more memory checks, but it's a topic for separate patch.

• link->num_cpus = 1;
• link->num_codecs = 1;
• if (strlen(pcm->pcm_name)) { link->name = kstrdup(pcm->pcm_name, GFP_KERNEL); link->stream_name = kstrdup(pcm->pcm_name,

GFP_KERNEL); @@ -1892,10 +1902,10 @@ static int soc_tplg_fe_link_create(struct soc_tplg *tplg, link->id = le32_to_cpu(pcm->pcm_id); if (strlen(pcm->dai_name))

• link->cpu_dai_name = kstrdup(pcm->dai_name,
  

GFP_KERNEL);

• link->cpus->dai_name = kstrdup(pcm->dai_name,
  

GFP_KERNEL);

• link->codec_name = "snd-soc-dummy";
• link->codec_dai_name = "snd-soc-dummy-dai";

• link->codecs->name = "snd-soc-dummy";

• link->codecs->dai_name = "snd-soc-dummy-dai";

  /* enable DPCM */ link->dynamic = 1;

@@ -1912,7 +1922,7 @@ static int soc_tplg_fe_link_create(struct soc_tplg *tplg, dev_err(tplg->comp->dev, "ASoC: FE link loading failed\n"); kfree(link->name); kfree(link->stream_name);

• kfree(link->cpu_dai_name);
  

• kfree(link->cpus->dai_name);
  

  kfree(link); return ret; }