On 8/26/13 4:53 PM, Zach Brown wrote:
>> With this we can
>> go through and convert any BUG_ON()'s that we have to catch actual 
>> programming
>> mistakes to the new ASSERT() and then fix everybody else to return errors.
> 
> I like the sound of that!
> 
>> --- a/fs/btrfs/ctree.h
>> +++ b/fs/btrfs/ctree.h
>> @@ -3814,6 +3814,22 @@ void btrfs_printk(const struct btrfs_fs_info 
>> *fs_info, const char *fmt, ...)
>>  #define btrfs_debug(fs_info, fmt, args...) \
>>      btrfs_printk(fs_info, KERN_DEBUG fmt, ##args)
>>  
>> +#ifdef BTRFS_ASSERT
>> +
>> +static inline void assfail(char *expr, char *file, int lin)
>> +{
>> +    printk(KERN_ERR "BTRFS assertion failed: %s, file: %s, line: %d",
>> +           expr, file, line);
>> +    BUG();
>> +}
> 
> I'm not sure why this is needed.

I think it's because we'd like to see the assertion that failed in plain text,
which then would need a function as above, but we'd rather not see that
_every_ ASSERT() failure was at the line of the BUG() in the helper function...

i.e. when xfs trips it does this:

XFS: Assertion failed: first <= last && last < BBTOB(bp->b_length),
file: fs/xfs/xfs_trans_buf.c, line: 568
------------[ cut here ]------------
kernel BUG at fs/xfs/xfs_message.c:108!

note the 2 different line numbers; 568 is what's relevant, 108 is not.

>> +#define ASSERT(expr)        \
>> +    (unlikely(expr) ? (void)0 : assfail(#expr, __FILE__, __LINE__))
> 
> (Passing the assertion is unlikely()?  I know, this is from xfs...
> still.)

hah, that's great.

>> +#else
>> +#define ASSERT(expr)        ((void)0)
>> +#endif
> 
> Anyway, if you're going to do it this way, why not:
> 
>       #ifdef BTRFS_ASSERT
>       #define btrfs_assert(cond)  BUG_ON(!(cond))
>       #else
>       #define btrfs_assert(cond)  do { if (cond) ; } while (0)
>       #endif

I think the only downside is that the BUG_ON() won't print the
conditional that failed, IIRC.

-Eric

> ?
> 
> - z

--
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