From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bruce Richardson Subject: Re: [PATCH] examples/distributor: fix missing "; " in debug macro Date: Tue, 23 Jun 2015 17:00:08 +0100 Message-ID: <20150623160007.GA2460@bricha3-MOBL3> References: <1433520077-11234-1-git-send-email-bruce.richardson@intel.com> <19910784.6F5piXXB0d@xps13> <20150608105810.GC3996@bricha3-MOBL3> <3471063.5PjCouUNn0@xps13> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dev@dpdk.org To: Thomas Monjalon Return-path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 6E0D55A95 for ; Tue, 23 Jun 2015 18:00:26 +0200 (CEST) Content-Disposition: inline In-Reply-To: <3471063.5PjCouUNn0@xps13> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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