Re: [dpdk-dev] [PATCH] examples/distributor: fix missing "; " in debug macro

[Bruce Richardson <bruce.richardson@intel.com>]

On Mon, Jun 22, 2015 at 10:53:25PM +0200, Thomas Monjalon wrote:
> 2015-06-08 11:58, Bruce Richardson:
> > On Fri, Jun 05, 2015 at 10:45:04PM +0200, Thomas Monjalon wrote:
> > > It shows that such dead code is almost never tested.
> > > It would be saner if this command would return no result:
> > > 	git grep 'ifdef.*DEBUG' examples
> > > 		examples/distributor/main.c:#ifdef DEBUG
> > > 		examples/l3fwd-acl/main.c:#ifdef L3FWDACL_DEBUG
> > > 		examples/l3fwd-acl/main.c:#ifdef L3FWDACL_DEBUG
> > > 		examples/l3fwd-acl/main.c:#ifdef L3FWDACL_DEBUG
> > > 		examples/l3fwd-acl/main.c:#ifdef L3FWDACL_DEBUG
> > > 		examples/packet_ordering/main.c:#ifdef DEBUG
> > > 		examples/vhost/main.c:#ifdef DEBUG
> > > 		examples/vhost/main.h:#ifdef DEBUG
> > > 		examples/vhost_xen/main.c:#ifdef DEBUG
> > > 		examples/vhost_xen/main.h:#ifdef DEBUG
> > > 
> > > There is no good reason to not use CONFIG_RTE_LOG_LEVEL to trigger debug build.
> > > 
> > I agree and disagree. 
> > 
> > I agree it would be good if we had a standard way of setting up
> > a DEBUG build that would make it easier to test and pick up on this sort of things.
> > 
> > I disagree that the compile time log level is the way to do this. The log level
> > at compile time specifies the default log level only, the actual log level is
> > controllable at runtime. Having the default log level also affect what kind of
> > build is done, e.g. with -O0 rather than -O3, introduces an unnecessary dependency.
> > Setting the default log level to 5 and changing it to 9 at runtime should be
> > the same as setting the default to 9.
> 
> Setting CONFIG_RTE_LOG_LEVEL to 9 means we don't care about performance
> degradation due to debug log branches. So it is necessarily a debug build.
> Then the default log level must be set by the application.
> The EAL default set from CONFIG_RTE_LOG_LEVEL is a last chance default in
> case the application doesn't care about it.
> 
> Maybe it won't convince you but anyway, it's not important here because the
> example applications don't use the DEBUG flag for anything else than the logs.
> That's why I think these flags must be removed.
> Please check "git grep 'ifdef.*DEBUG' examples".
> 

For checking if the app cares about performance, I would check the
__optimize__ define rather than having a specific DEBUG macro, or using the
DEFAULT_LOG_LEVEL setting.

/Bruce