Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755299Ab3CEJFe (ORCPT ); Tue, 5 Mar 2013 04:05:34 -0500 Received: from cantor2.suse.de ([195.135.220.15]:57576 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753536Ab3CEJFR (ORCPT ); Tue, 5 Mar 2013 04:05:17 -0500 Date: Tue, 05 Mar 2013 10:05:15 +0100 Message-ID: From: Takashi Iwai To: Christine Spang Cc: Jaroslav Kysela , alsa-devel@alsa-project.org, Jamie Iles , Sasha Levin , linux-kernel@vger.kernel.org Subject: Re: [PATCH] Make snd_BUG_ON() always evaluate and return the conditional expression. In-Reply-To: <1362434579-7733-1-git-send-email-christine.spang@oracle.com> References: <1362434579-7733-1-git-send-email-christine.spang@oracle.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.2 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3803 Lines: 101 At Mon, 4 Mar 2013 17:02:59 -0500, Christine Spang wrote: > > Having snd_BUG_ON() only evaluate its conditional when CONFIG_SND_DEBUG > is set leads to frequent bugs, since other similar macros in the kernel > have different behavior. Let's make snd_BUG_ON() act like those macros > so it will stop being accidentally misused. > > Signed-off-by: Christine Spang Sounds reasonable. The dependency on CONFIG_SND_DEBUG was for allowing more optimization, but since we use this for more places than expected, this change would be safer indeed. If no one has objection, I'll apply it for 3.10 kernel. thanks, Takashi > --- > Documentation/DocBook/writing-an-alsa-driver.tmpl | 12 +++++------- > include/sound/core.h | 24 ++++++++--------------- > 2 files changed, 13 insertions(+), 23 deletions(-) > > diff --git a/Documentation/DocBook/writing-an-alsa-driver.tmpl b/Documentation/DocBook/writing-an-alsa-driver.tmpl > index bd6fee2..06741e9 100644 > --- a/Documentation/DocBook/writing-an-alsa-driver.tmpl > +++ b/Documentation/DocBook/writing-an-alsa-driver.tmpl > @@ -6164,14 +6164,12 @@ struct _snd_pcm_runtime { > > > The macro takes an conditional expression to evaluate. > - When CONFIG_SND_DEBUG, is set, the > - expression is actually evaluated. If it's non-zero, it shows > - the warning message such as > + When CONFIG_SND_DEBUG, is set, if the > + expression is non-zero, it shows the warning message such as > BUG? (xxx) > - normally followed by stack trace. It returns the evaluated > - value. > - When no CONFIG_SND_DEBUG is set, this > - macro always returns zero. > + normally followed by stack trace. > + > + In both cases it returns the evaluated value. > > > > diff --git a/include/sound/core.h b/include/sound/core.h > index 7cede2d..a63680b 100644 > --- a/include/sound/core.h > +++ b/include/sound/core.h > @@ -379,18 +379,10 @@ void __snd_printk(unsigned int level, const char *file, int line, > * snd_BUG_ON - debugging check macro > * @cond: condition to evaluate > * > - * When CONFIG_SND_DEBUG is set, this macro evaluates the given condition, > - * and call WARN() and returns the value if it's non-zero. > - * > - * When CONFIG_SND_DEBUG is not set, this just returns zero, and the given > - * condition is ignored. > - * > - * NOTE: the argument won't be evaluated at all when CONFIG_SND_DEBUG=n. > - * Thus, don't put any statement that influences on the code behavior, > - * such as pre/post increment, to the argument of this macro. > - * If you want to evaluate and give a warning, use standard WARN_ON(). > + * Has the same behavior as WARN_ON when CONFIG_SND_DEBUG is set, > + * otherwise just evaluates the conditional and returns the value. > */ > -#define snd_BUG_ON(cond) WARN((cond), "BUG? (%s)\n", __stringify(cond)) > +#define snd_BUG_ON(cond) WARN_ON((cond)) > > #else /* !CONFIG_SND_DEBUG */ > > @@ -400,11 +392,11 @@ __printf(2, 3) > static inline void _snd_printd(int level, const char *format, ...) {} > > #define snd_BUG() do { } while (0) > -static inline int __snd_bug_on(int cond) > -{ > - return 0; > -} > -#define snd_BUG_ON(cond) __snd_bug_on(0 && (cond)) /* always false */ > + > +#define snd_BUG_ON(condition) ({ \ > + int __ret_warn_on = !!(condition); \ > + unlikely(__ret_warn_on); \ > +}) > > #endif /* CONFIG_SND_DEBUG */ > > -- > 1.8.2.rc0 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/