* [dpdk-dev] [PATCH] drivers/raw/ifpga_rawdev: fix coverity issue 323508
@ 2018-10-23 1:50 Rosen Xu
2018-10-23 6:59 ` Shreyansh Jain
2018-10-23 7:09 ` Shreyansh Jain
0 siblings, 2 replies; 8+ messages in thread
From: Rosen Xu @ 2018-10-23 1:50 UTC (permalink / raw)
To: dev; +Cc: tianfei.zhang, shreyansh.jain, hemant.agrawal, rosen.xu, ferruh.yigit
This patch fixes rte_eal_hotplug_add without checking return value issue
Signed-off-by: Rosen Xu <rosen.xu@intel.com>
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);
end:
if (kvlist)
@@ -591,7 +592,7 @@
if (name)
free(name);
- return 0;
+ return ret;
}
static int
--
1.8.3.1
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [PATCH] drivers/raw/ifpga_rawdev: fix coverity issue 323508 2018-10-23 1:50 [dpdk-dev] [PATCH] drivers/raw/ifpga_rawdev: fix coverity issue 323508 Rosen Xu @ 2018-10-23 6:59 ` Shreyansh Jain 2018-10-23 7:09 ` Shreyansh Jain 1 sibling, 0 replies; 8+ messages in thread From: Shreyansh Jain @ 2018-10-23 6:59 UTC (permalink / raw) To: Rosen Xu, dev; +Cc: tianfei.zhang, Hemant Agrawal, ferruh.yigit 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 <rosen.xu@intel.com> > Fixes: ef1e8ede3da5 ("raw/ifpga: add Intel FPGA bus rawdev driver") > Cc: rosen.xu@intel.com > --- Fixes comes *before* signed-off. <Patch header> <Message> .. <Fixes> <Signed-off...> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] drivers/raw/ifpga_rawdev: fix coverity issue 323508 2018-10-23 1:50 [dpdk-dev] [PATCH] drivers/raw/ifpga_rawdev: fix coverity issue 323508 Rosen Xu 2018-10-23 6:59 ` Shreyansh Jain @ 2018-10-23 7:09 ` Shreyansh Jain 2018-10-23 9:51 ` Ferruh Yigit 2018-10-25 10:25 ` Ferruh Yigit 1 sibling, 2 replies; 8+ messages in thread From: Shreyansh Jain @ 2018-10-23 7:09 UTC (permalink / raw) To: Rosen Xu, dev; +Cc: tianfei.zhang, Hemant Agrawal, ferruh.yigit 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 <rosen.xu@intel.com> > 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); 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 <shreyansh.jain@nxp.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] drivers/raw/ifpga_rawdev: fix coverity issue 323508 2018-10-23 7:09 ` Shreyansh Jain @ 2018-10-23 9:51 ` Ferruh Yigit 2018-10-23 10:43 ` Shreyansh Jain 2018-10-25 10:25 ` Ferruh Yigit 1 sibling, 1 reply; 8+ messages in thread From: Ferruh Yigit @ 2018-10-23 9:51 UTC (permalink / raw) To: Shreyansh Jain, Rosen Xu, dev; +Cc: tianfei.zhang, Hemant Agrawal 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 <rosen.xu@intel.com> >> 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 <shreyansh.jain@nxp.com> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] drivers/raw/ifpga_rawdev: fix coverity issue 323508 2018-10-23 9:51 ` Ferruh Yigit @ 2018-10-23 10:43 ` Shreyansh Jain 2018-10-23 12:14 ` Ferruh Yigit 0 siblings, 1 reply; 8+ messages in thread From: Shreyansh Jain @ 2018-10-23 10:43 UTC (permalink / raw) To: Ferruh Yigit, Rosen Xu; +Cc: dev, tianfei.zhang, Hemant Agrawal On Tuesday 23 October 2018 03:21 PM, Ferruh Yigit wrote: > 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 <rosen.xu@intel.com> >>> 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 > Oh!. Thanks - something I had missed reading. I don't want to hijack the conversation, but for my clarity, I think >>> snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "%d|%s", >>> port, name); won't be correct. Right? I am not suggesting that it should be changed now that it is already part of code. >> >> 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 <shreyansh.jain@nxp.com> >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] drivers/raw/ifpga_rawdev: fix coverity issue 323508 2018-10-23 10:43 ` Shreyansh Jain @ 2018-10-23 12:14 ` Ferruh Yigit 0 siblings, 0 replies; 8+ messages in thread From: Ferruh Yigit @ 2018-10-23 12:14 UTC (permalink / raw) To: Shreyansh Jain, Rosen Xu; +Cc: dev, tianfei.zhang, Hemant Agrawal On 10/23/2018 11:43 AM, Shreyansh Jain wrote: > On Tuesday 23 October 2018 03:21 PM, Ferruh Yigit wrote: >> 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 <rosen.xu@intel.com> >>>> 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 >> > > Oh!. Thanks - something I had missed reading. > > I don't want to hijack the conversation, but for my clarity, I think > > >>> snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "%d|%s", > >>> port, name); > > won't be correct. Right? You are right, it doesn't look correct. > I am not suggesting that it should be changed now that it is already > part of code. > >>> >>> 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 <shreyansh.jain@nxp.com> >>> >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] drivers/raw/ifpga_rawdev: fix coverity issue 323508 2018-10-23 7:09 ` Shreyansh Jain 2018-10-23 9:51 ` Ferruh Yigit @ 2018-10-25 10:25 ` Ferruh Yigit 2018-10-25 12:49 ` Xu, Rosen 1 sibling, 1 reply; 8+ messages in thread From: Ferruh Yigit @ 2018-10-25 10:25 UTC (permalink / raw) To: Shreyansh Jain, Rosen Xu, dev; +Cc: tianfei.zhang, Hemant Agrawal 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 <rosen.xu@intel.com> >> Fixes: ef1e8ede3da5 ("raw/ifpga: add Intel FPGA bus rawdev driver") >> Cc: rosen.xu@intel.com <...> > Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com> Coverity issue: 323508 Fixes: ef1e8ede3da5 ("raw/ifpga: add Intel FPGA bus rawdev driver") Cc: stable@dpdk.org Applied to dpdk-next-net/master, thanks. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] drivers/raw/ifpga_rawdev: fix coverity issue 323508 2018-10-25 10:25 ` Ferruh Yigit @ 2018-10-25 12:49 ` Xu, Rosen 0 siblings, 0 replies; 8+ messages in thread From: Xu, Rosen @ 2018-10-25 12:49 UTC (permalink / raw) To: Yigit, Ferruh, Shreyansh Jain, dev; +Cc: Zhang, Tianfei, Hemant Agrawal > -----Original Message----- > From: Yigit, Ferruh > Sent: Thursday, October 25, 2018 18:26 > To: Shreyansh Jain <shreyansh.jain@nxp.com>; Xu, Rosen > <rosen.xu@intel.com>; dev@dpdk.org > Cc: Zhang, Tianfei <tianfei.zhang@intel.com>; Hemant Agrawal > <hemant.agrawal@nxp.com> > Subject: Re: [dpdk-dev] [PATCH] drivers/raw/ifpga_rawdev: fix coverity issue > 323508 > > 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 <rosen.xu@intel.com> > >> Fixes: ef1e8ede3da5 ("raw/ifpga: add Intel FPGA bus rawdev driver") > >> Cc: rosen.xu@intel.com > > <...> > > Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com> > > Coverity issue: 323508 > Fixes: ef1e8ede3da5 ("raw/ifpga: add Intel FPGA bus rawdev driver") > Cc: stable@dpdk.org > > Applied to dpdk-next-net/master, thanks. Thanks all. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-10-25 12:49 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-23 1:50 [dpdk-dev] [PATCH] drivers/raw/ifpga_rawdev: fix coverity issue 323508 Rosen Xu 2018-10-23 6:59 ` Shreyansh Jain 2018-10-23 7:09 ` Shreyansh Jain 2018-10-23 9:51 ` Ferruh Yigit 2018-10-23 10:43 ` Shreyansh Jain 2018-10-23 12:14 ` Ferruh Yigit 2018-10-25 10:25 ` Ferruh Yigit 2018-10-25 12:49 ` Xu, Rosen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).