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

[MORITA Kazutaka]

At Thu, 11 Jul 2013 15:45:05 +0800,
Liu Yuan wrote:
> 
> 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

Okay, so your message
  "Don't include <assert.h>, use logger.h for assert()"
is for the users who include both assert.h and logger.h, right?  The
user already includes logger.h, so the message "use logger.h" looks a
bit strange.

"Don't include assert.h and logger.h in the same source file." looks
straightforward and don't mislead the developer who don't want to use
logger.h.

Thanks,

Kazutaka