[sheepdog] [PATCH] trace: exclude stuff related to tracing when trace is not enabled

[Liu Yuan]

On Sat, Jan 11, 2014 at 06:32:56PM +0900, Hitoshi Mitake wrote:
> At Fri, 10 Jan 2014 17:52:10 +0800,
> Liu Yuan wrote:
> > 
> > On Fri, Jan 10, 2014 at 02:18:28PM +0900, Hitoshi Mitake wrote:
> > > Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
> > > ---
> > >  dog/trace.c              |    1 +
> > >  include/Makefile.am      |    4 ++++
> > >  include/sheep.h          |   15 ---------------
> > >  include/sheepdog_trace.h |   19 +++++++++++++++++++
> > >  sheep/trace/trace.h      |    1 +
> > >  5 files changed, 25 insertions(+), 15 deletions(-)
> > >  create mode 100644 include/sheepdog_trace.h
> > > 
> > > diff --git a/dog/trace.c b/dog/trace.c
> > > index 806a3dd..51e061b 100644
> > > --- a/dog/trace.c
> > > +++ b/dog/trace.c
> > > @@ -23,6 +23,7 @@
> > >  #include "dog.h"
> > >  #include "rbtree.h"
> > >  #include "list.h"
> > > +#include "sheepdog_trace.h"
> > >  
> > >  static inline void print_thread_name(struct trace_graph_item *item)
> > >  {
> > > diff --git a/include/Makefile.am b/include/Makefile.am
> > > index 2c86984..13990bc 100644
> > > --- a/include/Makefile.am
> > > +++ b/include/Makefile.am
> > > @@ -4,3 +4,7 @@ noinst_HEADERS          = bitops.h event.h logger.h sheepdog_proto.h util.h \
> > >  			  list.h net.h sheep.h exits.h strbuf.h rbtree.h \
> > >  			  sha1.h option.h internal_proto.h shepherd.h work.h \
> > >  			  sockfd_cache.h compiler.h fec.h
> > > +
> > > +if BUILD_TRACE
> > > +noinst_HEADERS		+= sheepdog_trace.h
> > > +endif
> > > diff --git a/include/sheep.h b/include/sheep.h
> > > index d460d54..f7f5c48 100644
> > > --- a/include/sheep.h
> > > +++ b/include/sheep.h
> > > @@ -34,21 +34,6 @@ struct vnode_info {
> > >  	refcnt_t refcnt;
> > >  };
> > >  
> > > -#define TRACE_GRAPH_ENTRY  0x01
> > > -#define TRACE_GRAPH_RETURN 0x02
> > > -
> > > -#define TRACE_FNAME_LEN    36
> > > -#define TRACE_THREAD_LEN   MAX_THREAD_NAME_LEN
> > > -
> > > -struct trace_graph_item {
> > > -	char tname[TRACE_THREAD_LEN];
> > > -	int type;
> > > -	char fname[TRACE_FNAME_LEN];
> > > -	int depth;
> > > -	uint64_t entry_time;
> > > -	uint64_t return_time;
> > > -};
> > > -
> > 
> > No need to add another .h header, I think just add #ifdef here is okay. By the
> > way, does we really need to exclude stuff in the header, which doesn't decrease
> > the size of binary code at all.
> 
> The purpose of this patch is improvement of readability, not shrinking
> of the size of the binary. So I'm adding a new header instead of
> #ifdef.
> 

Why split this into a dedicated header help readability rather than add #ifdef if
you really care about it? I don't think these less than 20 lines will cause
readablility problem to people.

But in terms of convention we currently advertise, I think a better place for
this trace stuff should be include/internal_proto.h.

Thanks
Yuan