From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755462AbbHNPsG (ORCPT ); Fri, 14 Aug 2015 11:48:06 -0400 Received: from smtp102.biz.mail.bf1.yahoo.com ([98.139.221.61]:29053 "EHLO smtp102.biz.mail.bf1.yahoo.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755353AbbHNPsE (ORCPT ); Fri, 14 Aug 2015 11:48:04 -0400 X-Yahoo-Newman-Property: ymail-3 X-YMail-OSG: nQhQaroVM1nVP2cUoOcZILISZQ1U7ZTrl2N3RJQExBJo0nz nUvAXNzEyI1xThbe47wtbGCIMxvpPYVZ3LUmCAdwEvv_tVoK7a2MWHuR4I6C WOKS0M.MpU_Tpj77eJpE0m_QQPMX8_fdhl9zIOrXJCo46msLiTJ3u5u10dIe 7k45bOBRHpo0SF7bSbdZcDUYQSuDssLuTHq8hBCYm7Sc6vkENqpRhdl3Dfnr 1FmzWF0A3c7AteeBHzm6VlnCjyFKglgtdxEpsXWs2Y65jYa0j84JS_oV85Qo 9Kmvg26VkoCNQQE.6vaKY7lHSO37KNso1czn9co8ypRfQdmg4DxrOGpHowcF Tmpj.7kxt9DvUJQW8Okht.N51xeFWyDbgJYc62v5CKOGAGNhFVrFC9z9mUUG u5CZbrPMsV6xz7foXDIOCkNxVrm8RC7sQswIW8.h9SVTTT962HALRnKx2Yem VQK.TWhOdiD2ZYA3M6sRCj26LQbrC8ZCnslDSy2.rfChM7vf2J.2sLt9tUvY 6lgZTdrfQM9NNuTQzqLC6fQLam5axHBXa3A-- X-Yahoo-SMTP: OIJXglSswBDfgLtXluJ6wiAYv6_cnw-- Subject: Re: [RFC][PATCH v1 1/1] Add WARN_ONCE_RETURN macro To: Andy Shevchenko , Arnd Bergmann , linux-kernel@vger.kernel.org References: <1439559799-116694-1-git-send-email-andriy.shevchenko@linux.intel.com> Cc: Casey Schaufler From: Casey Schaufler Message-ID: <55CE0DB1.9060403@schaufler-ca.com> Date: Fri, 14 Aug 2015 08:48:01 -0700 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <1439559799-116694-1-git-send-email-andriy.shevchenko@linux.intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 8/14/2015 6:43 AM, Andy Shevchenko wrote: > The rationale is to replace the > if (condition) { > WARN_ONCE(1, ...); > return value; > } > by > WARN_ONCE_RETURN(condition, value, ...); The kernel coding style says: Things to avoid when using macros: 1) macros that affect control flow: #define FOO(x) \ do { \ if (blah(x) < 0) \ return -EBUGGERED; \ } while(0) is a _very_ bad idea. It looks like a function call but exits the "calling" function; don't break the internal parsers of those who will read the code. So no, I won't be taking this unless there's a change in the coding guidelines, which I coincidentally agree with. > > Signed-off-by: Andy Shevchenko > --- > include/asm-generic/bug.h | 17 +++++++++++++++++ > security/smack/smack_lsm.c | 5 +---- > 2 files changed, 18 insertions(+), 4 deletions(-) > > diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h > index 630dd23..fbf20fd 100644 > --- a/include/asm-generic/bug.h > +++ b/include/asm-generic/bug.h > @@ -126,6 +126,18 @@ extern void warn_slowpath_null(const char *file, const int line); > unlikely(__ret_warn_once); \ > }) > > +#define WARN_ONCE_RETURN(condition, value, format...) \ > +do { \ > + static bool __section(.data.unlikely) __warned; \ > + int __ret_warn_once = !!(condition); \ > + \ > + if (unlikely(__ret_warn_once)) { \ > + if (WARN(!__warned, format)) \ > + __warned = true; \ > + return value; \ > + } \ > +} while (0) > + > #define WARN_TAINT_ONCE(condition, taint, format...) ({ \ > static bool __section(.data.unlikely) __warned; \ > int __ret_warn_once = !!(condition); \ > @@ -162,6 +174,11 @@ extern void warn_slowpath_null(const char *file, const int line); > > #define WARN_ON_ONCE(condition) WARN_ON(condition) > #define WARN_ONCE(condition, format...) WARN(condition, format) > +#define WARN_ONCE_RETURN(condition, value, format...) \ > +do { \ > + if (WARN(condition, format)) \ > + return value; \ > +} while (0) > #define WARN_TAINT(condition, taint, format...) WARN(condition, format) > #define WARN_TAINT_ONCE(condition, taint, format...) WARN(condition, format) > > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c > index 54fb3a1..4160d0e 100644 > --- a/security/smack/smack_lsm.c > +++ b/security/smack/smack_lsm.c > @@ -4401,10 +4401,7 @@ static int smack_audit_rule_match(u32 secid, u32 field, u32 op, void *vrule, > struct smack_known *skp; > char *rule = vrule; > > - if (unlikely(!rule)) { > - WARN_ONCE(1, "Smack: missing rule\n"); > - return -ENOENT; > - } > + WARN_ONCE_RETURN(unlikely(!rule), -ENOENT, "Smack: missing rule\n"); > > if (field != AUDIT_SUBJ_USER && field != AUDIT_OBJ_USER) > return 0;