* [RFC][PATCH v1 1/1] Add WARN_ONCE_RETURN macro
@ 2015-08-14 13:43 Andy Shevchenko
2015-08-14 15:48 ` Casey Schaufler
0 siblings, 1 reply; 3+ messages in thread
From: Andy Shevchenko @ 2015-08-14 13:43 UTC (permalink / raw)
To: Arnd Bergmann, Casey Schaufler, linux-kernel; +Cc: Andy Shevchenko
The rationale is to replace the
if (condition) {
WARN_ONCE(1, ...);
return value;
}
by
WARN_ONCE_RETURN(condition, value, ...);
Signed-off-by: Andy Shevchenko <andriy.shevchenko@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;
--
2.5.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC][PATCH v1 1/1] Add WARN_ONCE_RETURN macro
2015-08-14 13:43 [RFC][PATCH v1 1/1] Add WARN_ONCE_RETURN macro Andy Shevchenko
@ 2015-08-14 15:48 ` Casey Schaufler
2015-08-14 18:25 ` Andy Shevchenko
0 siblings, 1 reply; 3+ messages in thread
From: Casey Schaufler @ 2015-08-14 15:48 UTC (permalink / raw)
To: Andy Shevchenko, Arnd Bergmann, linux-kernel; +Cc: Casey Schaufler
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 <andriy.shevchenko@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;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC][PATCH v1 1/1] Add WARN_ONCE_RETURN macro
2015-08-14 15:48 ` Casey Schaufler
@ 2015-08-14 18:25 ` Andy Shevchenko
0 siblings, 0 replies; 3+ messages in thread
From: Andy Shevchenko @ 2015-08-14 18:25 UTC (permalink / raw)
To: Casey Schaufler
Cc: Andy Shevchenko, Arnd Bergmann, linux-kernel@vger.kernel.org
On Fri, Aug 14, 2015 at 6:48 PM, Casey Schaufler <casey@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.shevchenko@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 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/
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-08-14 18:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-14 13:43 [RFC][PATCH v1 1/1] Add WARN_ONCE_RETURN macro Andy Shevchenko
2015-08-14 15:48 ` Casey Schaufler
2015-08-14 18:25 ` Andy Shevchenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.