From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 40A911B20B for ; Fri, 14 Dec 2018 20:20:40 +0100 (CET) X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Dec 2018 11:20:39 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,354,1539673200"; d="scan'208";a="302265070" Received: from ae13-28.jf.intel.com ([10.166.188.62]) by fmsmga006.fm.intel.com with ESMTP; 14 Dec 2018 11:20:38 -0800 Date: Fri, 14 Dec 2018 11:17:06 -0800 From: Jeff Shaw To: Stephen Hemminger Cc: Jeff Shaw , dev@dpdk.org, hofors@lysator.liu.se Message-ID: <20181214191706.GA10186@ae13-28.jf.intel.com> References: <20181214163827.9403-1-jeffrey.b.shaw@intel.com> <20181214103603.59336e0e@xeon-e3> <20181214185928.GA9964@ae13-28.jf.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20181214185928.GA9964@ae13-28.jf.intel.com> User-Agent: Mutt/1.9.2 (2017-12-15) Subject: Re: [dpdk-dev] [PATCH] eal: remove variable length array X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 14 Dec 2018 19:20:40 -0000 On Fri, Dec 14, 2018 at 10:59:28AM -0800, Jeff Shaw wrote: > On Fri, Dec 14, 2018 at 10:36:03AM -0800, Stephen Hemminger wrote: > > On Fri, 14 Dec 2018 08:38:27 -0800 > > Jeff Shaw wrote: > > > > > Compilers that do not support the C11 standard, or do not implement > > > gcc extensions, may not support variable length arrays. > > > > > > The code prior to this commit produced the following warning when > > > compiled with "-Wvla -std=c90". > > > > > > warning: ISO C90 forbids variable length array ‘array’ [-Wvla] > > > > > > This commit removes the variable length array from the PMD debug > > > trace function by allocating memory dynamically on the stack using > > > alloca(). > > > > > > Signed-off-by: Jeff Shaw > > > --- > > > lib/librte_eal/common/include/rte_dev.h | 19 +++++++++---------- > > > 1 file changed, 9 insertions(+), 10 deletions(-) > > > > > > diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h > > > index a9724dc91..af772872b 100644 > > > --- a/lib/librte_eal/common/include/rte_dev.h > > > +++ b/lib/librte_eal/common/include/rte_dev.h > > > @@ -47,22 +47,21 @@ __attribute__((format(printf, 2, 0))) > > > static inline void > > > rte_pmd_debug_trace(const char *func_name, const char *fmt, ...) > > > { > > > + char *buffer; > > > + int buf_len; > > > va_list ap; > > > > > > va_start(ap, fmt); > > > + buf_len = vsnprintf(NULL, 0, fmt, ap) + 1; > > > + va_end(ap); > > > > > > - { > > > - char buffer[vsnprintf(NULL, 0, fmt, ap) + 1]; > > > + buffer = (char *)alloca(buf_len); > > > > alloca is void * so cast is not necessary. > > I get a compiler warning on Windows, hence the cast. I suppose we could > disable the warning. > > > > > You might be able to skip the buffering step entirely by using rte_log > > a little more creatively, see how other logging works. > > Probably right... this eventuall needs to be done in ~100 other places, > most of which aren't used with rte_log(), so it seemed fitting. > > > > > But to go further since rte_pmd_debug_trace is not used anywhere in > > current code base in DPDK, it should just be removed. > > Much better solution :) Actually, it cannot be removed. It is used by librte_eventdev. When RTE_LIBRTE_EVENTDEV_DEBUG is defined, the RTE_PMD_DEBUG_TRACE macro uses rte_pmd_debug_trace. I will remove the cast and update the commit message to reference C99 instead of C11 as requested by Mattias R.