From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 87F62A046B for ; Wed, 21 Aug 2019 10:58:48 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 36A951BEE7; Wed, 21 Aug 2019 10:58:47 +0200 (CEST) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id 9ACA01BEE5 for ; Wed, 21 Aug 2019 10:58:45 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 21 Aug 2019 01:58:44 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,412,1559545200"; d="scan'208";a="169353365" Received: from irsmsx108.ger.corp.intel.com ([163.33.3.3]) by orsmga007.jf.intel.com with ESMTP; 21 Aug 2019 01:58:43 -0700 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.7]) by IRSMSX108.ger.corp.intel.com ([169.254.11.50]) with mapi id 14.03.0439.000; Wed, 21 Aug 2019 09:58:42 +0100 From: "Van Haaren, Harry" To: Stephen Hemminger CC: "dev@dpdk.org" Thread-Topic: [PATCH] service: print errors to rte log Thread-Index: AQHVV6+pClUzq1C9hEaSt6l0bd8zoqcFS+vw Date: Wed, 21 Aug 2019 08:58:42 +0000 Message-ID: References: <20190820233256.27405-1-stephen@networkplumber.org> In-Reply-To: <20190820233256.27405-1-stephen@networkplumber.org> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiOTM0ZjRlZWEtNDRmNS00YzkyLWIzNTEtZDBjNGVkOTc5NGZiIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiUEtBQjBGU3Q2d1ZhRGFBbXlENDc0cFdGWDQ0dW1IWGpFbXV0S2ZHNVVHRXFRcjV3TktFXC9VbmhmMFR3Nm4xb3EifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] service: print errors to rte log 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > -----Original Message----- > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > Sent: Wednesday, August 21, 2019 12:33 AM > To: Van Haaren, Harry > Cc: dev@dpdk.org; Stephen Hemminger > Subject: [PATCH] service: print errors to rte log >=20 > EAL should always use rte_log instead of putting errors to > stderr (which maybe redirected to /dev/null in a daemon). >=20 > Also checks for null before rte_free are unnecessary. >=20 > Signed-off-by: Stephen Hemminger Thanks - good improvements. A few nit-picks, I'll send a v2 based on your changes here with the below notes implemented. I'll add my Sign-off for code changes, and Acked-by for the whole, hope that's OK, if you'd prefer two different patches just let me know. -H > --- > lib/librte_eal/common/rte_service.c | 23 +++++++++++------------ > 1 file changed, 11 insertions(+), 12 deletions(-) >=20 > diff --git a/lib/librte_eal/common/rte_service.c > b/lib/librte_eal/common/rte_service.c > index c3653ebae46c..aa2f8f3ef4b1 100644 > --- a/lib/librte_eal/common/rte_service.c > +++ b/lib/librte_eal/common/rte_service.c > @@ -70,10 +70,12 @@ static struct rte_service_spec_impl *rte_services; > static struct core_state *lcore_states; > static uint32_t rte_service_library_initialized; >=20 > + > int32_t rte_service_init(void) > { Added line here should really split return-value and function into two lines. Found another instance of this, splitting that too to make the whole file consistent. Rest of file uses 1 line to split variable declarations and functions, so one line will do. > if (!rte_services) { > - printf("error allocating rte services array\n"); > + RTE_LOG(ERR, EAL, > + "error allocating rte services array\n"); > goto fail_mem; Some of these "strings" can be on the same line as RTE_LOG and stay inside the 80 char limit, moving them up a line for consistency.