[3/5] ASoC: topology: use break on errors, not continue

Commit Message

Pierre-Louis Bossart July 7, 2020, 8:37 p.m. UTC

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 <[e-mail]>
Reviewed-by: Ranjani Sridharan <[e-mail]>
Reviewed-by: Kai Vehmanen <[e-mail]>
---
 sound/soc/soc-topology.c | 53 +++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 25 deletions(-)

Patch

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 */