On 12/05/2016 09:08 PM, Qu Wenruo wrote: > > > At 12/06/2016 10:51 AM, Goldwyn Rodrigues wrote: >> >> >> On 12/05/2016 08:03 PM, Qu Wenruo wrote: >>> BTW, the DISABLE_BACKTRACE branch seems quite different from >>> backtrace one. >>> >>> #define BUG_ON(c) assert_trace(#c, __FILE__, __func__, __LINE__, >>> (long)(c)) >>> #define WARN_ON(c) warning_trace(#c, __FILE__, __func__, __LINE__, >>> (long)(c)) >>> #define ASSERT(c) assert_trace(#c, __FILE__, __func__, __LINE__, >>> (long)!(c)) >>> #define BUG() assert_trace(NULL, __FILE__, __func__, __LINE__, 1) >>> #else >>> #define BUG_ON(c) assert(!(c)) >>> #define WARN_ON(c) warning_trace(#c, __FILE__, __func__, __LINE__, >>> (long)(c)) >>> #define ASSERT(c) assert(!(c)) >>> #define BUG() assert(0) >>> >>> Condition of BUG_ON/ASSERT/BUG are all logical notted for >>> DISABLE_BACKTRACE. >>> While WARN_ON() of both branch are the same condition. >> >> WARN_ON is using warning_trace as opposed to assert, and that is the >> reason it is not notted. >> >>> >>> This seems quite confusing to me. >>> >>> Any idea to make it more straightforward? >>> >> >> I just kept it the same as before. warning_trace was using an extra >> variable, trace, which was not needed because the print_trace was >> already in ifndefs. > > I mean, better make the condition the same for both BUG/BUG_ON/ASSERT. > So that we don't need to manually logical not the condition. First of all, ASSERT and BUG_ON have opposite meanings. ASSERT() checks if the condition is true and continues (halts if false). BUG_ON() "bugs" if condition is true and halts (continues if false). So you would have to use opposite conditions. > > For example: > #define ASSERT(c) assert_trace(#c, __FILE__, __func__, __LINE__,（long） > (c)) > and > #define ASSERT(c) assert((c)) > > This looks much more straightforward, and easier to expose bug at review > time. Could you explain with a patch? You idea seems to add more code than reduce it. -- Goldwyn -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
