* [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).