alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: alsa-devel@alsa-project.org
Cc: tiwai@suse.de, broonie@kernel.org,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
	Kai Vehmanen <kai.vehmanen@linux.intel.com>,
	Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Subject: [PATCH 3/5] ASoC: topology: use break on errors, not continue
Date: Tue,  7 Jul 2020 15:37:47 -0500	[thread overview]
Message-ID: <20200707203749.113883-4-pierre-louis.bossart@linux.intel.com> (raw)
In-Reply-To: <20200707203749.113883-1-pierre-louis.bossart@linux.intel.com>

Since the beginning of the topology, the code continues to the next
object even when an error is detected.

The topology should be handled with an all-or-nothing design, loading
a partially valid topology is a sure way to get bug reports that are
difficult to deal with.

Changing the behavior may break previous solutions and expose problems
in topology files delivered in the past, so it's probably not wise to
add this patch to stable branches without revalidation.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 sound/soc/soc-topology.c | 53 +++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 25 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 6eaa00c21011..d42f73f7038f 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -741,7 +741,8 @@ static int soc_tplg_dbytes_create(struct soc_tplg *tplg, unsigned int count,
 	struct snd_soc_tplg_bytes_control *be;
 	struct soc_bytes_ext *sbe;
 	struct snd_kcontrol_new kc;
-	int i, err;
+	int i;
+	int err = 0;
 
 	if (soc_tplg_check_elem_count(tplg,
 		sizeof(struct snd_soc_tplg_bytes_control), count,
@@ -786,7 +787,7 @@ static int soc_tplg_dbytes_create(struct soc_tplg *tplg, unsigned int count,
 		if (err) {
 			soc_control_err(tplg, &be->hdr, be->hdr.name);
 			kfree(sbe);
-			continue;
+			break;
 		}
 
 		/* pass control to driver for optional further init */
@@ -796,7 +797,7 @@ static int soc_tplg_dbytes_create(struct soc_tplg *tplg, unsigned int count,
 			dev_err(tplg->dev, "ASoC: failed to init %s\n",
 				be->hdr.name);
 			kfree(sbe);
-			continue;
+			break;
 		}
 
 		/* register control here */
@@ -806,12 +807,12 @@ static int soc_tplg_dbytes_create(struct soc_tplg *tplg, unsigned int count,
 			dev_err(tplg->dev, "ASoC: failed to add %s\n",
 				be->hdr.name);
 			kfree(sbe);
-			continue;
+			break;
 		}
 
 		list_add(&sbe->dobj.list, &tplg->comp->dobj_list);
 	}
-	return 0;
+	return err;
 
 }
 
@@ -821,7 +822,8 @@ static int soc_tplg_dmixer_create(struct soc_tplg *tplg, unsigned int count,
 	struct snd_soc_tplg_mixer_control *mc;
 	struct soc_mixer_control *sm;
 	struct snd_kcontrol_new kc;
-	int i, err;
+	int i;
+	int err = 0;
 
 	if (soc_tplg_check_elem_count(tplg,
 		sizeof(struct snd_soc_tplg_mixer_control),
@@ -880,7 +882,7 @@ static int soc_tplg_dmixer_create(struct soc_tplg *tplg, unsigned int count,
 		if (err) {
 			soc_control_err(tplg, &mc->hdr, mc->hdr.name);
 			kfree(sm);
-			continue;
+			break;
 		}
 
 		/* create any TLV data */
@@ -889,7 +891,7 @@ static int soc_tplg_dmixer_create(struct soc_tplg *tplg, unsigned int count,
 			dev_err(tplg->dev, "ASoC: failed to create TLV %s\n",
 				mc->hdr.name);
 			kfree(sm);
-			continue;
+			break;
 		}
 
 		/* pass control to driver for optional further init */
@@ -900,7 +902,7 @@ static int soc_tplg_dmixer_create(struct soc_tplg *tplg, unsigned int count,
 				mc->hdr.name);
 			soc_tplg_free_tlv(tplg, &kc);
 			kfree(sm);
-			continue;
+			break;
 		}
 
 		/* register control here */
@@ -911,13 +913,13 @@ static int soc_tplg_dmixer_create(struct soc_tplg *tplg, unsigned int count,
 				mc->hdr.name);
 			soc_tplg_free_tlv(tplg, &kc);
 			kfree(sm);
-			continue;
+			break;
 		}
 
 		list_add(&sm->dobj.list, &tplg->comp->dobj_list);
 	}
 
-	return 0;
+	return err;
 }
 
 static int soc_tplg_denum_create_texts(struct soc_enum *se,
@@ -997,7 +999,8 @@ static int soc_tplg_denum_create(struct soc_tplg *tplg, unsigned int count,
 	struct snd_soc_tplg_enum_control *ec;
 	struct soc_enum *se;
 	struct snd_kcontrol_new kc;
-	int i, ret, err;
+	int i;
+	int err = 0;
 
 	if (soc_tplg_check_elem_count(tplg,
 		sizeof(struct snd_soc_tplg_enum_control),
@@ -1053,7 +1056,7 @@ static int soc_tplg_denum_create(struct soc_tplg *tplg, unsigned int count,
 					"ASoC: could not create values for %s\n",
 					ec->hdr.name);
 				kfree(se);
-				continue;
+				goto err_denum;
 			}
 			/* fall through */
 		case SND_SOC_TPLG_CTL_ENUM:
@@ -1065,15 +1068,16 @@ static int soc_tplg_denum_create(struct soc_tplg *tplg, unsigned int count,
 					"ASoC: could not create texts for %s\n",
 					ec->hdr.name);
 				kfree(se);
-				continue;
+				goto err_denum;
 			}
 			break;
 		default:
+			err = -EINVAL;
 			dev_err(tplg->dev,
 				"ASoC: invalid enum control type %d for %s\n",
 				ec->hdr.ops.info, ec->hdr.name);
 			kfree(se);
-			continue;
+			goto err_denum;
 		}
 
 		/* map io handlers */
@@ -1081,7 +1085,7 @@ static int soc_tplg_denum_create(struct soc_tplg *tplg, unsigned int count,
 		if (err) {
 			soc_control_err(tplg, &ec->hdr, ec->hdr.name);
 			kfree(se);
-			continue;
+			goto err_denum;
 		}
 
 		/* pass control to driver for optional further init */
@@ -1091,23 +1095,23 @@ static int soc_tplg_denum_create(struct soc_tplg *tplg, unsigned int count,
 			dev_err(tplg->dev, "ASoC: failed to init %s\n",
 				ec->hdr.name);
 			kfree(se);
-			continue;
+			goto err_denum;
 		}
 
 		/* register control here */
-		ret = soc_tplg_add_kcontrol(tplg,
-			&kc, &se->dobj.control.kcontrol);
-		if (ret < 0) {
+		err = soc_tplg_add_kcontrol(tplg,
+					    &kc, &se->dobj.control.kcontrol);
+		if (err < 0) {
 			dev_err(tplg->dev, "ASoC: could not add kcontrol %s\n",
 				ec->hdr.name);
 			kfree(se);
-			continue;
+			goto err_denum;
 		}
 
 		list_add(&se->dobj.list, &tplg->comp->dobj_list);
 	}
-
-	return 0;
+err_denum:
+	return err;
 }
 
 static int soc_tplg_kcontrol_elems_load(struct soc_tplg *tplg,
@@ -1361,8 +1365,7 @@ static struct snd_kcontrol_new *soc_tplg_dapm_widget_dmixer_create(
 		if (err < 0) {
 			dev_err(tplg->dev, "ASoC: failed to create TLV %s\n",
 				mc->hdr.name);
-			kfree(sm);
-			continue;
+			goto err_sm;
 		}
 
 		/* pass control to driver for optional further init */
-- 
2.25.1


  parent reply	other threads:[~2020-07-07 20:42 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-07 20:37 [PATCH 0/5] ASoC: topology: fix error handling flow Pierre-Louis Bossart
2020-07-07 20:37 ` [PATCH 1/5] ASoC: topology: fix kernel oops on route addition error Pierre-Louis Bossart
2020-07-07 20:37 ` [PATCH 2/5] ASoC: topology: fix tlvs in error handling for widget_dmixer Pierre-Louis Bossart
2020-07-07 20:37 ` Pierre-Louis Bossart [this message]
2020-07-07 20:37 ` [PATCH 4/5] ASoC: topology: factor kfree(se) in error handling Pierre-Louis Bossart
2020-07-07 20:37 ` [PATCH 5/5] ASoC: topology: add more logs when topology load fails Pierre-Louis Bossart
2020-07-08 17:00 ` [PATCH 0/5] ASoC: topology: fix error handling flow Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200707203749.113883-4-pierre-louis.bossart@linux.intel.com \
    --to=pierre-louis.bossart@linux.intel.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=kai.vehmanen@linux.intel.com \
    --cc=ranjani.sridharan@linux.intel.com \
    --cc=tiwai@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).