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 07148A046B for ; Wed, 26 Jun 2019 13:39:35 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 98EE0F64; Wed, 26 Jun 2019 13:39:34 +0200 (CEST) Received: from mail-io1-f67.google.com (mail-io1-f67.google.com [209.85.166.67]) by dpdk.org (Postfix) with ESMTP id DE6DD2AB for ; Wed, 26 Jun 2019 13:39:32 +0200 (CEST) Received: by mail-io1-f67.google.com with SMTP id k20so4195387ios.10 for ; Wed, 26 Jun 2019 04:39:32 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Uy6GhkFibvZ0I5O2u1oOhy4ipwHCt2fZ4VY91Cbaen0=; b=a6lwXcq4TFuFLPPx6k1BsYAeIBd4VvUVODV5RbSf0X26VgewQuL/q2cj2axZ0u2EH0 P1VKKrChT3IFvPFchkoQ3cV2/dtuJMFHBbc8dSt0Q2j0saZclQ0tUh0HdHc0UGsekxxI mLJU8YtGVBahun8xmduAVEFcUViaoF1EohAF5KZic/HpzNIINJ2bXlbL7fSChw3hbxKW jZvk144EjrbDhDHYbczWsD0Jj0OkCDmUyylxAWvNPpuO/qkzoTMepI76604nU79gNfma DjfbqkBdkqbRlinxGAXFjjX8aA6EdQ+9VCjmrxWgGLyL/GFotVZZWM2DLLDCVHsfWjdo 5y0A== X-Gm-Message-State: APjAAAX8r52zHcOCV4TD015q42QgdynH0bo0nd2vHJVcYEFKSgTk8PMh LnAr4+JQiJOE6HcdBF+IwUN0wjKTYqbtV3cGK6V+dgry5Ec= X-Google-Smtp-Source: APXvYqx1W3Mz6qCBv3lTDTFFH7ZB//J0kB/+7FfU01yAlco0HlV438Vkyooun8pXozf+7maYD0eDHfhsQHfahO+2qWI= X-Received: by 2002:a5e:9314:: with SMTP id k20mr4570430iom.235.1561549171827; Wed, 26 Jun 2019 04:39:31 -0700 (PDT) MIME-Version: 1.0 References: <20190626104056.26829-1-thomas@monjalon.net> <2203940.txDa9y0fLx@xps> In-Reply-To: <2203940.txDa9y0fLx@xps> From: David Marchand Date: Wed, 26 Jun 2019 13:39:20 +0200 Message-ID: To: Thomas Monjalon Cc: dev Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 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" 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; > After saying these arguments, I have to say I have no strong opinion :) > I'm fine either way. > Reviewed-by: David Marchand I won't insist either, if you feel like taking my proposal, you can keep the reviewed-by token. -- David Marchand