From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by inbox.dpdk.org (Postfix) with ESMTP id DA89CA046B
for <public@inbox.dpdk.org>; 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 <dev@dpdk.org>; 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: <xms:TF0TXQJwM9Chc18eWv5VfIcTlYs2FqreCyunxtCIl2mTAP5cKB4pew>
X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduvddrudeigdegkecutefuodetggdotefrodftvf
curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu
uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc
fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs
ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucfkph
epjeejrddufeegrddvtdefrddukeegnecurfgrrhgrmhepmhgrihhlfhhrohhmpehthhho
mhgrshesmhhonhhjrghlohhnrdhnvghtnecuvehluhhsthgvrhfuihiivgeptd
X-ME-Proxy: <xmx:TV0TXRb-OXagdL_gzvLdLCREaC3zAaQjiAzGWPJIC5EjOf2Q6rLeNA>
<xmx:TV0TXWCu4A7fAJ2Tx0mhEtRbmYvtz0Q1aI2niBSwcxXT2gBuJEyrxQ>
<xmx:TV0TXbUQDQJxlt1VALNOJAyJDPrdMh9U5dDq96iPvtxoXElibgCthg>
<xmx:TV0TXe29QMaHArKzQ72MPO8wxtkkAlloJYL97yc-QP8gxFfitycQKQ>
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 <thomas@monjalon.net>
To: "Burakov, Anatoly" <anatoly.burakov@intel.com>
Cc: David Marchand <david.marchand@redhat.com>, dev <dev@dpdk.org>
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>
<CAJFAV8wmMwEptNhM8X-jSwrSA6d4Aean_c53LP3=6L7zLpPnaQ@mail.gmail.com>
<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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
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 <thomas@monjalon.net> wrote:
> >
> >> 26/06/2019 13:20, David Marchand:
> >>> On Wed, Jun 26, 2019 at 12:41 PM Thomas Monjalon <thomas@monjalon.net>
> >>> 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.