[sheepdog] [PATCH v2 1/4] util: solve cyclic dependency of util.h and logger.h

[Liu Yuan]

On Tue, Jul 09, 2013 at 05:56:44PM +0900, Hitoshi Mitake wrote:
> At Tue, 9 Jul 2013 16:51:04 +0800,
> Liu Yuan wrote:
> > 
> > On Tue, Jul 09, 2013 at 04:42:00PM +0900, Hitoshi Mitake wrote:
> > > Current sheepdog specialized assert() in util.h cannot be used by
> > > util.h itself because it depends on logger.h and logger.h depends on
> > > util.h, too.
> > > 
> > > This patch solves this cyclic dependency.
> > > 
> > > Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
> > > ---
> > > 
> > > v2:
> > >  - stop let treeview.c and shadow_file.c include logger.h
> > > 
> > >  include/logger.h |    6 ++++++
> > >  include/util.h   |   39 ++++++++++++++++++++++++---------------
> > >  2 files changed, 30 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/include/logger.h b/include/logger.h
> > > index 5c9aab6..41a1488 100644
> > > --- a/include/logger.h
> > > +++ b/include/logger.h
> > > @@ -81,4 +81,10 @@ 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();
> > > +}
> > > +
> > >  #endif	/* LOG_H */
> > > diff --git a/include/util.h b/include/util.h
> > > index e70bb9f..1517680 100644
> > > --- a/include/util.h
> > > +++ b/include/util.h
> > > @@ -14,6 +14,23 @@
> > >  #include "bitops.h"
> > >  #include "list.h"
> > >  
> > > +static inline void do_assert(const char *expr, const char *func, int line);
> > > +
> > > +#ifdef assert
> > > +#undef assert
> > > +#endif
> > > +
> > > +#ifndef NDEBUG
> > > +
> > > +#define assert(expr) ((expr) ?				\
> > > +			(void)0 : do_assert(#expr, __func__, __LINE__))
> > > +
> > > +#else
> > > +
> > > +#define assert(expr) ((void)0)
> > > +
> > > +#endif	/* NDEBUG */
> > > +
> > >  #define SECTOR_SIZE (1U << 9)
> > >  #define BLOCK_SIZE (1U << 12)
> > >  
> > > @@ -161,21 +178,6 @@ int atomic_create_and_write(const char *path, char *buf, size_t len);
> > >  	__removed;							\
> > >  })
> > >  
> > > -#ifdef assert
> > > -#undef assert
> > > -#endif
> > > -
> > > -#ifndef NDEBUG
> > > -
> > > -#define assert(expr) ((expr) ?						\
> > > -			(void)0 : panic("Asserting `%s' failed.", #expr))
> > > -
> > > -#else
> > > -
> > > -#define assert(expr) ((void)0)
> > > -
> > > -#endif	/* NDEBUG */
> > > -
> > >  /* urcu helpers */
> > >  
> > >  /* Boolean data type which can be accessed by multiple threads */
> > > @@ -243,4 +245,11 @@ extern mode_t sd_def_dmode;
> > >  /* Force a compilation error if the condition is true */
> > >  #define BUILD_BUG_ON(condition) ((void)sizeof(struct { int: -!!(condition); }))
> > >  
> > > +/*
> > > + * The below including of logger.h is required for suppressing compiler warning
> > > + * because if .c file include util.h but not logger.h, the prototype
> > > + * declaration of do_assert() is treated as defined but not used function.
> > > + */
> > > +#include "logger.h"
> > > +
> > >  #endif
> > 
> > This looks too hacky to me, can we find a better method?
> 
> I tried to seek other methods (e.g. using gcc extensions) but
> concluded this solution is an only way to solve the problem.
> 
> I think this one is not so hacky because almost every .c file includes
> both of util.h and logger.h. treeview.c and shadow_files.c are only
> exceptions.

What is the problem this patch tries to solve? Not every one including 'util.h'
want to use assert(). and we don't have problems in the old code, do we?

Thanks
Yuan