At 12/06/2016 08:44 PM, Goldwyn Rodrigues wrote:


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.

I know, I mean, for both backtrace disabled and enabled case, the condition should be the same.

If not the same condition, it means assert_trace() has different meaning than original assert.



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.

Sure, will submit one soon.

Thanks,
Qu


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to