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 DA89CA046B for ; Wed, 26 Jun 2019 13:55:59 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A54B01E25; Wed, 26 Jun 2019 13:55:59 +0200 (CEST) Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by dpdk.org (Postfix) with ESMTP id F1481DED for ; Wed, 26 Jun 2019 13:55:57 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 7C7D92217D; Wed, 26 Jun 2019 07:55:57 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Wed, 26 Jun 2019 07:55:57 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=GdgRqoyp13ZDbWFI/JicWMry40qeB20HMFy53BuKAAc=; b=oAEmqtVQeIpK INzgJeOgOzVzqIeE1NS5IrN+YxkOXXlHdwMceWeMjyomTkp3YXTQyhslz1j1ehJn Dz1YJYtq8YVCwoIn4GQlf+Y7JiKFc9oQn3vpmreuThnpHky2gSX9grK+1lJPAX67 jGe9G+MiOTV3yr+hv5RT83+hBbMhssQ= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; bh=GdgRqoyp13ZDbWFI/JicWMry40qeB20HMFy53BuKA Ac=; b=Xq9zWstx20y0SAbh+s3fskLUytILlU1zALKhVJB8Gi19AaYoQSXTNlRyq +zpzp7+LtSBHf7m2b9HgwMY2DWA3H9EYASBbw1aefFrVp9/jFb40kOAcNpnzyfG/ wXkh9lGO5Jr/Ww8oTMZQQUBSniqWYh8ypTYkebiuEAGZU0aBm8RLarzY/ZDOlBIA qsYL7SZsw4ehQcYbe2i1zGvHipJqf8NpOOYY3rz1MpvBu8IkaMUqeKK+svci+1gQ LceLY9v+jKDRx8+MyKzuB4/UV0syy1mlbM+ENPeCNWHtG/2EOAGtbbL6OHd+JPhQ lzQKrTKYRX0TnBoMZuG3rH5WPM8nw== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduvddrudeigdegkecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucfkph epjeejrddufeegrddvtdefrddukeegnecurfgrrhgrmhepmhgrihhlfhhrohhmpehthhho mhgrshesmhhonhhjrghlohhnrdhnvghtnecuvehluhhsthgvrhfuihiivgeptd X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 7D4D280063; Wed, 26 Jun 2019 07:55:56 -0400 (EDT) From: Thomas Monjalon To: "Burakov, Anatoly" Cc: David Marchand , dev Date: Wed, 26 Jun 2019 13:55:53 +0200 Message-ID: <8628822.145IoVqhEV@xps> In-Reply-To: <305b244d-ef0a-937b-9628-94aaede96b27@intel.com> References: <20190626104056.26829-1-thomas@monjalon.net> <305b244d-ef0a-937b-9628-94aaede96b27@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH] eal/linux: fix return after alarm registration failure 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" 26/06/2019 13:43, Burakov, Anatoly: > On 26-Jun-19 12:39 PM, David Marchand wrote: > > On Wed, Jun 26, 2019 at 1:36 PM Thomas Monjalon wrote: > > > >> 26/06/2019 13:20, David Marchand: > >>> On Wed, Jun 26, 2019 at 12:41 PM Thomas Monjalon > >>> wrote: > >>> > >>>> When adding an alarm, if an error happen when registering > >>>> the common alarm callback, it is not considered as a major failure. > >>>> The alarm is then inserted in the list. > >>>> However it was returning an error code after inserting the alarm. > >>>> > >>>> The error code is reset to 0 so the behaviour and the return code > >>>> are consistent. > >>>> Other return code related lines are cleaned up for easier > >> understanding. > >>>> > >> [...] > >>>> --- a/lib/librte_eal/linux/eal/eal_alarm.c > >>>> +++ b/lib/librte_eal/linux/eal/eal_alarm.c > >>>> if (!handler_registered) { > >>>> - ret |= rte_intr_callback_register(&intr_handle, > >>>> + ret = rte_intr_callback_register(&intr_handle, > >>>> eal_alarm_callback, NULL); > >>>> - handler_registered = (ret == 0) ? 1 : 0; > >>>> + if (ret == 0) > >>>> + handler_registered = 1; > >>>> + else > >>>> + /* not fatal, callback can be registered later > >> */ > >>>> + ret = 0; > >>>> } > >>> > >>> Well, then it means that you don't want to touch ret at all. > >>> How about: > >>> if (rte_intr_callback_register(&intr_handle, > >>> eal_alarm_callback, NULL) == 0) > >>> handler_registered = 1; > >>> > >>> ? > >> > >> Too much simple :) > >> > >> I think we try to avoid calling a function in a "if" > >> per coding style. > >> And my proposal has the benefit of offering a comment > >> about the non-fatal error. > >> > > > > /* not fatal, callback can be registered later */ > > if (rte_intr_callback_register(&intr_handle, > > eal_alarm_callback, NULL) == 0) > > handler_registered = 1; > > > > I prefer the original. It's more explicit and conveys the intention > better. Did i break the tie? :) I was going to send a v2 with David's suggestion. Now I'm confused.