From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 89579A0C47; Thu, 2 Sep 2021 12:30:19 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 440F640041; Thu, 2 Sep 2021 12:30:19 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mails.dpdk.org (Postfix) with ESMTP id 086534003C for ; Thu, 2 Sep 2021 12:30:16 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10094"; a="280075916" X-IronPort-AV: E=Sophos;i="5.84,372,1620716400"; d="scan'208";a="280075916" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Sep 2021 03:30:16 -0700 X-IronPort-AV: E=Sophos;i="5.84,372,1620716400"; d="scan'208";a="532984763" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.1.171]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 02 Sep 2021 03:30:14 -0700 Date: Thu, 2 Sep 2021 11:30:11 +0100 From: Bruce Richardson To: Mattias =?iso-8859-1?Q?R=F6nnblom?= Cc: dev@dpdk.org, conor.walsh@intel.com, kevin.laatz@intel.com, fengchengwen@huawei.com, jerinj@marvell.com Message-ID: References: <20210826183301.333442-1-bruce.richardson@intel.com> <20210901163216.120087-1-bruce.richardson@intel.com> <20210901163216.120087-3-bruce.richardson@intel.com> <4ee0f186-47c5-63cc-63ad-df71b5df0f1f@lysator.liu.se> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <4ee0f186-47c5-63cc-63ad-df71b5df0f1f@lysator.liu.se> Subject: Re: [dpdk-dev] [PATCH v2 2/6] app/test: add basic dmadev instance tests X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, Sep 01, 2021 at 09:24:12PM +0200, Mattias Rönnblom wrote: > On 2021-09-01 18:32, Bruce Richardson wrote: > > Run basic sanity tests for configuring, starting and stopping a dmadev > > instance to help validate drivers. This also provides the framework for > > future tests for data-path operation. > > > > Signed-off-by: Bruce Richardson > > --- > > app/test/test_dmadev.c | 81 ++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 81 insertions(+) > > > > diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c > > index bb01e86483..12f7c69629 100644 > > --- a/app/test/test_dmadev.c > > +++ b/app/test/test_dmadev.c > > @@ -2,6 +2,7 @@ > > * Copyright(c) 2021 HiSilicon Limited. > > * Copyright(c) 2021 Intel Corporation. > > */ > > +#include > > #include > > #include > > @@ -13,6 +14,77 @@ > > /* from test_dmadev_api.c */ > > extern int test_dmadev_api(uint16_t dev_id); > > +#define PRINT_ERR(...) print_err(__func__, __LINE__, __VA_ARGS__) > > + > > +static inline int > > Remove inline. > While I understand it's probably not doing a lot having "inline" there, any particular reason why you think it should be removed? > > +__rte_format_printf(3, 4) > > +print_err(const char *func, int lineno, const char *format, ...) > > +{ > > + va_list ap; > > + int ret; > > + > > + ret = fprintf(stderr, "In %s:%d - ", func, lineno); > Check return code here, and return on error. > > > + va_start(ap, format); > > + ret += vfprintf(stderr, format, ap); > > ..and here. > > > + va_end(ap); > > + > > + return ret; > > A negative return value in one call and an valid byte count result for the > other should produce an error, but here it might not. > > You might argue this is just test code, but then I suggest not checking the > return values at all. > Indeed the return value is never checked anywhere in the calls to PRINT_ERR macro, and since the writes are going to stderr it's pretty low risk. Therefore, I'll remove the return value handling completely as you suggest. /Bruce