Skip to content

Commit b4d83c8

Browse files
author
Ingo Molnar
committed
headers/cleanup.h: Remove the if_not_guard() facility
Linus noticed that the new if_not_guard() definition is fragile: "This macro generates actively wrong code if it happens to be inside an if-statement or a loop without a block. IOW, code like this: for (iterate-over-something) if_not_guard(a) return -BUSY; looks like will build fine, but will generate completely incorrect code." The reason is that the __if_not_guard() macro is multi-statement, so while most kernel developers expect macros to be simple or at least compound statements - but for __if_not_guard() it is not so: #define __if_not_guard(_name, _id, args...) \ BUILD_BUG_ON(!__is_cond_ptr(_name)); \ CLASS(_name, _id)(args); \ if (!__guard_ptr(_name)(&_id)) To add insult to injury, the placement of the BUILD_BUG_ON() line makes the macro appear to compile fine, but it will generate incorrect code as Linus reported, for example if used within iteration or conditional statements that will use the first statement of a macro as a loop body or conditional statement body. [ I'd also like to note that the original submission by David Lechner did not contain the BUILD_BUG_ON() line, so it was safer than what we ended up committing. Mea culpa. ] It doesn't appear to be possible to turn this macro into a robust single or compound statement that could be used in single statements, due to the necessity to define an auto scope variable with an open scope and the necessity of it having to expand to a partial 'if' statement with no body. Instead of trying to work around this fragility, just remove the construct before it gets used. Reported-by: Linus Torvalds <[email protected]> Signed-off-by: Ingo Molnar <[email protected]> Cc: David Lechner <[email protected]> Cc: Peter Zijlstra <[email protected]> Link: https://lore.kernel.org/r/[email protected]
1 parent 0302d2f commit b4d83c8

File tree

1 file changed

+0
-14
lines changed

1 file changed

+0
-14
lines changed

include/linux/cleanup.h

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -273,12 +273,6 @@ static inline class_##_name##_t class_##_name##ext##_constructor(_init_args) \
273273
* an anonymous instance of the (guard) class, not recommended for
274274
* conditional locks.
275275
*
276-
* if_not_guard(name, args...) { <error handling> }:
277-
* convenience macro for conditional guards that calls the statement that
278-
* follows only if the lock was not acquired (typically an error return).
279-
*
280-
* Only for conditional locks.
281-
*
282276
* scoped_guard (name, args...) { }:
283277
* similar to CLASS(name, scope)(args), except the variable (with the
284278
* explicit name 'scope') is declard in a for-loop such that its scope is
@@ -350,14 +344,6 @@ _label: \
350344
#define scoped_cond_guard(_name, _fail, args...) \
351345
__scoped_cond_guard(_name, _fail, __UNIQUE_ID(label), args)
352346

353-
#define __if_not_guard(_name, _id, args...) \
354-
BUILD_BUG_ON(!__is_cond_ptr(_name)); \
355-
CLASS(_name, _id)(args); \
356-
if (!__guard_ptr(_name)(&_id))
357-
358-
#define if_not_guard(_name, args...) \
359-
__if_not_guard(_name, __UNIQUE_ID(guard), args)
360-
361347
/*
362348
* Additional helper macros for generating lock guards with types, either for
363349
* locks that don't have a native type (eg. RCU, preempt) or those that need a

0 commit comments

Comments
 (0)