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 <[email protected]>
> ---
> 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
