Re: [PATCH] Btrfs: add support for asserts

[Josef Bacik <jbacik@fusionio.com>]

On Mon, Aug 26, 2013 at 02:53:26PM -0700, 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.
> 
> > +#define ASSERT(expr)	\
> > +	(unlikely(expr) ? (void)0 : assfail(#expr, __FILE__, __LINE__))
> 
> (Passing the assertion is unlikely()?  I know, this is from xfs...
> still.)
>

Yeah I copy+pasted and then thought about it after I sent it.  I will fix it up.
 
> > +#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 like the verbosity, especially with random kernel versions and such, it will
help me figure out where we BUG_ON()'ed without having to checkout a particular
version and go hunting.  Thanks,

Josef