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
