From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id B00DB592F for ; Mon, 2 May 2016 15:37:13 +0200 (CEST) Received: from was59-1-82-226-113-214.fbx.proxad.net ([82.226.113.214] helo=[192.168.0.10]) by mail.droids-corp.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84_2) (envelope-from ) id 1axE48-0004p5-4R; Mon, 02 May 2016 15:39:12 +0200 To: Daniel Mrzyglod , dev@dpdk.org References: <1460638879-45680-1-git-send-email-danielx.t.mrzyglod@intel.com> From: Olivier Matz Message-ID: <57275802.7090606@6wind.com> Date: Mon, 2 May 2016 15:37:06 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.6.0 MIME-Version: 1.0 In-Reply-To: <1460638879-45680-1-git-send-email-danielx.t.mrzyglod@intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] cmdline: fix unchecked return value X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 02 May 2016 13:37:13 -0000 Hi Daniel, On 04/14/2016 03:01 PM, Daniel Mrzyglod wrote: > This patch is for checking if error values occurs. > fix for coverity errors #13209 & #13195 > > If the function returns an error value, the error value may be mistaken > for a normal value. > > In rdline_char_in: Value returned from a function is not checked for errors > before being used > > Signed-off-by: Daniel Mrzyglod > --- > lib/librte_cmdline/cmdline_rdline.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/lib/librte_cmdline/cmdline_rdline.c b/lib/librte_cmdline/cmdline_rdline.c > index 1ef2258..e75a556 100644 > --- a/lib/librte_cmdline/cmdline_rdline.c > +++ b/lib/librte_cmdline/cmdline_rdline.c > @@ -377,7 +377,10 @@ rdline_char_in(struct rdline *rdl, char c) > case CMDLINE_KEY_CTRL_K: > cirbuf_get_buf_head(&rdl->right, rdl->kill_buf, RDLINE_BUF_SIZE); > rdl->kill_size = CIRBUF_GET_LEN(&rdl->right); > - cirbuf_del_buf_head(&rdl->right, rdl->kill_size); > + > + if (cirbuf_del_buf_head(&rdl->right, rdl->kill_size) < 0) > + return -EINVAL; > + > rdline_puts(rdl, vt100_clear_right); > break; > I wonder if a better way to fix wouldn't be to remove the checks introduced in http://dpdk.org/browse/dpdk/commit/?id=ab971e562860 There is no reason to check that in cirbuf_get_buf_head/tail(): if (!cbuf || !c) The function should never fail, it just returns the number of copied chars. This is the responsibility of the caller to ensure that the pointer to the circular buffer is not NULL. Also, rdline_char_in() is not expected to return -EINVAL, but RDLINE_RES_* instead. So I think that partially revert ab971e562860 would fix the coverity warning. Regards, Olivier