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 47063A046B for ; Wed, 26 Jun 2019 13:36:32 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 7A778F64; Wed, 26 Jun 2019 13:36:31 +0200 (CEST) Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by dpdk.org (Postfix) with ESMTP id ECEC2DED for ; Wed, 26 Jun 2019 13:36:29 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 876282205C; Wed, 26 Jun 2019 07:36:29 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Wed, 26 Jun 2019 07:36:29 -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=zMuKxSVXSBgSbQQzBpeVIkBJ+Zm7V+v8kCKCQxV1QgU=; b=D0dqToiE1SrK 1+XH6BkgeGmFonEKt4A5m+5Aw/a0wQDsDRW6Z6nc6NuFUnjpwfoaRxAwPscFJrn0 73IEcGAFJWhXsTJB/wILplXcUE2v25sdkDvP91MUm5wUM3g5TYHvFHKfklbMfYRJ NM3D94QZgxr0N3KnbTjn3Adwn8Gjr50= 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=zMuKxSVXSBgSbQQzBpeVIkBJ+Zm7V+v8kCKCQxV1Q gU=; b=ZOBKJcRrvuuLtslOApjMxyF11aV1Nu9EdwgleJZHSqzzctxVN3hZybWZo AXoIvKwsgZn01KyutV8cpThs9A0DoxAlPq73WScqhfFgB834W/beMnwtIvFYa84d 5+5ouT4Hp4T0oNzdLq5LKz71FcHT87APBOadnGJpn1Q8NagoR1Jyr63LslkRPc1k cXeV+T5ZdDwkwc6xkVeUpSdBAyUvId5htr2pVHE3PRxadA6h73K5QjnBQy24YHSI nJtSq7S+p9RF9FtGDjbVQ6XR93mjikuOL4bB9SVEDck5jDwUWXnoFHW4Z5StCvzx vLBWEKde1mCqDFRkpQk5WKXth34eQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduvddrudeigdeggecutefuodetggdotefrodftvf 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 D1275380085; Wed, 26 Jun 2019 07:36:28 -0400 (EDT) From: Thomas Monjalon To: David Marchand Cc: dev Date: Wed, 26 Jun 2019 13:36:26 +0200 Message-ID: <2203940.txDa9y0fLx@xps> In-Reply-To: References: <20190626104056.26829-1-thomas@monjalon.net> 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: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. After saying these arguments, I have to say I have no strong opinion :) I'm fine either way.