From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH 21/31] ASoC: dapm: Use WARN_ON() instead of BUG_ON() Date: Wed, 06 Nov 2013 12:05:00 +0100 Message-ID: References: <1383673218-18405-1-git-send-email-tiwai@suse.de> <1383673218-18405-22-git-send-email-tiwai@suse.de> <20131106102431.GM11602@sirena.org.uk> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 41091261664 for ; Wed, 6 Nov 2013 12:05:01 +0100 (CET) In-Reply-To: <20131106102431.GM11602@sirena.org.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Mark Brown Cc: alsa-devel@alsa-project.org, Liam Girdwood List-Id: alsa-devel@alsa-project.org At Wed, 6 Nov 2013 10:24:31 +0000, Mark Brown wrote: > > On Tue, Nov 05, 2013 at 06:40:08PM +0100, Takashi Iwai wrote: > > > list_for_each_entry(w, pending, power_list) { > > - BUG_ON(reg != w->reg); > > + if (WARN_ON(reg != w->reg)) > > + return; > > w->power = w->new_power; > > This doesn't seem at all sensible - it's going to suddenly abort the > entire sequence run over the error which if we're actually trying to > continue is just going to make things worse. I'd expect to at most skip > over the entry. OK, what about this one? Takashi --- From: Takashi Iwai Subject: [PATCH v2] ASoC: dapm: Use WARN_ON() instead of BUG_ON() BUG_ON() is rather useless for debugging as it leads to panic(). Use WARN_ON() and handle the error cases accordingly. Also another WARN_ON() check is added in dapm_seq_run_coalesced() since now the show goes on after the first WARN_ON(). Signed-off-by: Takashi Iwai --- sound/soc/soc-dapm.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index cc36caaf6443..d191a1aee432 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -1460,7 +1460,8 @@ static void dapm_seq_run_coalesced(struct snd_soc_card *card, power_list)->reg; list_for_each_entry(w, pending, power_list) { - BUG_ON(reg != w->reg); + if (WARN_ON(reg != w->reg)) + continue; w->power = w->new_power; mask |= w->mask << w->shift; @@ -1493,6 +1494,8 @@ static void dapm_seq_run_coalesced(struct snd_soc_card *card, } list_for_each_entry(w, pending, power_list) { + if (WARN_ON(reg != w->reg)) + continue; dapm_seq_check_event(card, w, SND_SOC_DAPM_POST_PMU); dapm_seq_check_event(card, w, SND_SOC_DAPM_POST_PMD); } @@ -3359,8 +3362,10 @@ static int snd_soc_dai_link_event(struct snd_soc_dapm_widget *w, u64 fmt; int ret; - BUG_ON(!config); - BUG_ON(list_empty(&w->sources) || list_empty(&w->sinks)); + if (WARN_ON(!config)) + return -EINVAL; + if (WARN_ON(list_empty(&w->sources) || list_empty(&w->sinks))) + return -EINVAL; /* We only support a single source and sink, pick the first */ source_p = list_first_entry(&w->sources, struct snd_soc_dapm_path, @@ -3368,9 +3373,12 @@ static int snd_soc_dai_link_event(struct snd_soc_dapm_widget *w, sink_p = list_first_entry(&w->sinks, struct snd_soc_dapm_path, list_source); - BUG_ON(!source_p || !sink_p); - BUG_ON(!sink_p->source || !source_p->sink); - BUG_ON(!source_p->source || !sink_p->sink); + if (WARN_ON(!source_p || !sink_p)) + return -EINVAL; + if (WARN_ON(!sink_p->source || !source_p->sink)) + return -EINVAL; + if (WARN_ON(!source_p->source || !sink_p->sink)) + return -EINVAL; source = source_p->source->priv; sink = sink_p->sink->priv; -- 1.8.4.2