From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ferruh Yigit Subject: Re: [PATCH] drivers/raw/ifpga_rawdev: fix coverity issue 323508 Date: Tue, 23 Oct 2018 10:51:09 +0100 Message-ID: References: <1540259449-135300-1-git-send-email-rosen.xu@intel.com> <74cb8aff-f809-b999-8c2e-17f24599d827@nxp.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: "tianfei.zhang@intel.com" , Hemant Agrawal To: Shreyansh Jain , Rosen Xu , "dev@dpdk.org" Return-path: Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id D36CD1B44A for ; Tue, 23 Oct 2018 11:51:11 +0200 (CEST) In-Reply-To: <74cb8aff-f809-b999-8c2e-17f24599d827@nxp.com> Content-Language: en-US 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 10/23/2018 8:09 AM, Shreyansh Jain wrote: > Besides the comment I sent before about 'Fixes' before sign-off, a > single trivial comment inline ... > > On Tuesday 23 October 2018 07:20 AM, Rosen Xu wrote: >> This patch fixes rte_eal_hotplug_add without checking return value issue >> >> Signed-off-by: Rosen Xu >> Fixes: ef1e8ede3da5 ("raw/ifpga: add Intel FPGA bus rawdev driver") >> Cc: rosen.xu@intel.com >> --- >> drivers/raw/ifpga_rawdev/ifpga_rawdev.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/raw/ifpga_rawdev/ifpga_rawdev.c b/drivers/raw/ifpga_rawdev/ifpga_rawdev.c >> index 3fed057..32e318f 100644 >> --- a/drivers/raw/ifpga_rawdev/ifpga_rawdev.c >> +++ b/drivers/raw/ifpga_rawdev/ifpga_rawdev.c >> @@ -542,6 +542,7 @@ >> int port; >> char *name = NULL; >> char dev_name[RTE_RAWDEV_NAME_MAX_LEN]; >> + int ret = -1; >> >> devargs = dev->device.devargs; >> >> @@ -583,7 +584,7 @@ >> snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "%d|%s", >> port, name); >> >> - rte_eal_hotplug_add(RTE_STR(IFPGA_BUS_NAME), >> + ret = rte_eal_hotplug_add(RTE_STR(IFPGA_BUS_NAME), >> dev_name, devargs->args); > > Ideally, the function argument spreading on next line should start > underneath the previous arguments - something like: > > ret = rte_eal_hotplug_add(RTE_STR(IFPGA_BUS_NAME), > dev_name, devargs->args); Hi Shreyansh, According dpdk coding convention [1], indentation done by hard tab, code seems inline with coding convention, only perhaps can be done single tab instead of double. And to remind again, I am not for syntax discussions but just defining one and consistently follow it . [1] https://doc.dpdk.org/guides/contributing/coding_style.html#c-indentation https://doc.dpdk.org/guides/contributing/coding_style.html#prototypes > > But, in this file this is not being done at multiple places (for > example, the snprintf in this code snippet). So, either you can ignore > this comment, or fix it for just this change. > >> end: >> if (kvlist) >> @@ -591,7 +592,7 @@ >> if (name) >> free(name); >> >> - return 0; >> + return ret; >> } >> >> static int >> > > Otherwise, the patch is simple enough. > > Acked-by: Shreyansh Jain >