[sheepdog] [PATCH] move assert definition from util.h to logger.h

[Liu Yuan]

On Thu, Jul 11, 2013 at 04:31:44PM +0900, MORITA Kazutaka wrote:
> At Thu, 11 Jul 2013 12:55:18 +0800,
> Liu Yuan wrote:
> > 
> > On Thu, Jul 11, 2013 at 01:11:57PM +0900, MORITA Kazutaka wrote:
> > > If we don't use a logger, we don't need to define our assert.  In that
> > > sense, assert() should be defined in logger.h.  The only problem is
> > > that assert can be used before including logger.h.  To avoid it, this
> > > patch adds an #error directive to detect assert definition before
> > > logger.h.
> > > 
> > > This fixes a compiler error which happens when NDEBUG is not defined.
> > > 
> > > Signed-off-by: MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp>
> > > ---
> > >  include/compiler.h       |  3 +++
> > >  include/logger.h         | 14 +++++++++-----
> > >  include/sheep.h          |  2 +-
> > >  include/sheepdog_proto.h |  3 ++-
> > >  include/util.h           | 24 +++++-------------------
> > >  lib/event.c              |  2 +-
> > >  lib/net.c                |  2 +-
> > >  lib/util.c               |  3 +--
> > >  lib/work.c               |  2 +-
> > >  sheep/sheep_priv.h       |  1 +
> > >  sheepfs/volume.c         |  1 -
> > >  shepherd/shepherd.c      |  1 -
> > >  12 files changed, 25 insertions(+), 33 deletions(-)
> > > 
> > > diff --git a/include/compiler.h b/include/compiler.h
> > > index 1cd2440..b2ae002 100644
> > > --- a/include/compiler.h
> > > +++ b/include/compiler.h
> > > @@ -17,4 +17,7 @@
> > >  
> > >  #define __printf(a, b) __attribute__((format(printf, a, b)))
> > >  
> > > +/* Force a compilation error if the condition is true */
> > > +#define BUILD_BUG_ON(condition) ((void)sizeof(struct { int: -!!(condition); }))
> > > +
> > >  #endif	/* SD_COMPILER_H */
> > > diff --git a/include/logger.h b/include/logger.h
> > > index c2c4237..9bc3bbe 100644
> > > --- a/include/logger.h
> > > +++ b/include/logger.h
> > > @@ -81,10 +81,14 @@ void sd_backtrace(void);
> > >  		log_write(SDOG_DEBUG, __func__, __LINE__, fmt, ##args);	\
> > >  	} while (0)
> > >  
> > > -static inline void do_assert(const char *expr, const char *func, int line)
> > > -{
> > > -	log_write(SDOG_EMERG, func, line, "Asserting `%s' failed.", expr);
> > > -	abort();
> > > -}
> > > +#ifdef assert
> > > +#error "logger.h must be included before assert.h"
> > 
> > I think we should always use sheepdog's assert(). So this would be better rephrased
> > as "Don't include <assert.h>, use logger.h for assert()"
> 
> I thought that at least sheepfs doesn't want to include logger.h.  Is
> it wrong?  Our sd_printf() and assert() print a thread name and it is
> meaningless for the programs who don't use worker threads.
> 

I don't see the conflict. This error only is fired when people include <assert.h>
and logger.h in the same source file. sheepfs doesn't include logger.h.

I don't see the point both include assert.h and logger.h in the same source file

Thanks
Yuan