On Fri, Aug 14, 2015 at 6:48 PM, Casey Schaufler <ca...@schaufler-ca.com> wrote:
> 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.
My intuition tells me something like that, that's why RFC (and on
Friday). I didn't think it's already clarified in the CosdingStyle.
Thanks for remind about this nice document.
And sorry for a noise.
>
> 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 <andriy.shevche...@linux.intel.com>
>> ---
>> 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;
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
