* [dpdk-dev] [PATCH] net/ark: fix for Coverity issues
@ 2017-05-11 11:02 John Miller
2017-05-12 11:11 ` Ferruh Yigit
0 siblings, 1 reply; 3+ messages in thread
From: John Miller @ 2017-05-11 11:02 UTC (permalink / raw)
To: dev; +Cc: shepard.siegel, ed.czeck, John Miller
Fixes: 9c7188a68d7b ("net/ark: provide API for hardware modules pktchkr and pktgen")
Coverity issue: 144513
Fixes: 727b3fe292bc ("net/ark: integrate PMD")
Coverity issue: 144514
Fixes: 9c7188a68d7b ("net/ark: provide API for hardware modules pktchkr and pktgen")
Coverity issue: 144512
Fixes: 1131cbf0fb2b ("net/ark: stub PMD for Atomic Rules Arkville")
Coverity issue: 144517
Fixes: 727b3fe292bc ("net/ark: integrate PMD")
Coverity issue: 144520
Signed-off-by: John Miller <john.miller@atomicrules.com>
---
drivers/net/ark/ark_ethdev.c | 18 ++++++++++++------
drivers/net/ark/ark_pktchkr.c | 3 ++-
drivers/net/ark/ark_pktgen.c | 3 ++-
3 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ark/ark_ethdev.c b/drivers/net/ark/ark_ethdev.c
index 995c93d..5f00a02 100644
--- a/drivers/net/ark/ark_ethdev.c
+++ b/drivers/net/ark/ark_ethdev.c
@@ -516,11 +516,7 @@ static void eth_ark_macaddr_remove(struct rte_eth_dev *dev,
dev->dev_ops = NULL;
dev->rx_pkt_burst = NULL;
dev->tx_pkt_burst = NULL;
- if (dev->data->mac_addrs)
- rte_free(dev->data->mac_addrs);
- if (dev->data)
- rte_free(dev->data);
-
+ rte_free(dev->data->mac_addrs);
return 0;
}
@@ -588,7 +584,11 @@ static void eth_ark_macaddr_remove(struct rte_eth_dev *dev,
/* Delay packet generatpr start allow the hardware to be ready
* This is only used for sanity checking with internal generator
*/
- pthread_create(&thread, NULL, delay_pg_start, ark);
+ if (pthread_create(&thread, NULL, delay_pg_start, ark)) {
+ PMD_DRV_LOG(ERR, "Could not create pktgen "
+ "starter thread\n");
+ return -1;
+ }
}
if (ark->user_ext.dev_start)
@@ -899,6 +899,12 @@ static void eth_ark_macaddr_remove(struct rte_eth_dev *dev,
int size = 0;
int first = 1;
+ if (file == NULL) {
+ PMD_DRV_LOG(ERR, "Unable to open, "
+ "config file %s\n", value);
+ return -1;
+ }
+
while (fgets(line, sizeof(line), file)) {
size += strlen(line);
if (size >= ARK_MAX_ARG_LEN) {
diff --git a/drivers/net/ark/ark_pktchkr.c b/drivers/net/ark/ark_pktchkr.c
index 62b3673..e933026 100644
--- a/drivers/net/ark/ark_pktchkr.c
+++ b/drivers/net/ark/ark_pktchkr.c
@@ -372,7 +372,8 @@ struct OPTIONS {
o->v.INT = atoll(val);
break;
case OTSTRING:
- strncpy(o->v.STR, val, ARK_MAX_STR_LEN);
+ strncpy(o->v.STR, val, ARK_MAX_STR_LEN - 1);
+ o->v.STR[ARK_MAX_STR_LEN - 1] = 0;
break;
}
return 1;
diff --git a/drivers/net/ark/ark_pktgen.c b/drivers/net/ark/ark_pktgen.c
index bdac054..7308f14 100644
--- a/drivers/net/ark/ark_pktgen.c
+++ b/drivers/net/ark/ark_pktgen.c
@@ -354,7 +354,8 @@ struct OPTIONS {
o->v.INT = atoll(val);
break;
case OTSTRING:
- strncpy(o->v.STR, val, ARK_MAX_STR_LEN);
+ strncpy(o->v.STR, val, ARK_MAX_STR_LEN - 1);
+ o->v.STR[ARK_MAX_STR_LEN - 1] = 0;
break;
}
return 1;
--
1.9.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [dpdk-dev] [PATCH] net/ark: fix for Coverity issues
2017-05-11 11:02 [dpdk-dev] [PATCH] net/ark: fix for Coverity issues John Miller
@ 2017-05-12 11:11 ` Ferruh Yigit
2017-05-13 11:28 ` john miller
0 siblings, 1 reply; 3+ messages in thread
From: Ferruh Yigit @ 2017-05-12 11:11 UTC (permalink / raw)
To: John Miller, dev; +Cc: shepard.siegel, ed.czeck
On 5/11/2017 12:02 PM, John Miller wrote:
> Fixes: 9c7188a68d7b ("net/ark: provide API for hardware modules pktchkr and pktgen")
> Coverity issue: 144513
>
> Fixes: 727b3fe292bc ("net/ark: integrate PMD")
> Coverity issue: 144514
>
> Fixes: 9c7188a68d7b ("net/ark: provide API for hardware modules pktchkr and pktgen")
> Coverity issue: 144512
>
> Fixes: 1131cbf0fb2b ("net/ark: stub PMD for Atomic Rules Arkville")
> Coverity issue: 144517
The convention is Coverity line first, Fixes line later.
>
> Fixes: 727b3fe292bc ("net/ark: integrate PMD")
> Coverity issue: 144520
Hi John,
Thanks for fixing coverity issues.
Can you please split patch into a patchset with multiple patches,
grouped to same kind of fixes?
And instead of having "coverity fix" in patch title, can you please
describe what is really fixed, like "fix not null terminated buffer" or
"fix missing function return check" etc ...
Thanks,
ferruh
> Signed-off-by: John Miller <john.miller@atomicrules.com>
<...>
> --- a/drivers/net/ark/ark_pktgen.c
> +++ b/drivers/net/ark/ark_pktgen.c
> @@ -354,7 +354,8 @@ struct OPTIONS {
> o->v.INT = atoll(val);
> break;
> case OTSTRING:
> - strncpy(o->v.STR, val, ARK_MAX_STR_LEN);
> + strncpy(o->v.STR, val, ARK_MAX_STR_LEN - 1);
> + o->v.STR[ARK_MAX_STR_LEN - 1] = 0;
This also works, but you can prefer to switch snprintf(), which
guaranties the null termination.
> break;
> }
> return 1;
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [dpdk-dev] [PATCH] net/ark: fix for Coverity issues
2017-05-12 11:11 ` Ferruh Yigit
@ 2017-05-13 11:28 ` john miller
0 siblings, 0 replies; 3+ messages in thread
From: john miller @ 2017-05-13 11:28 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: dev, Shepard Siegel, ed.czeck
Hi Ferruh,
Thank you for your review.
I will create a new patchset for these fixes with all of the changes you requested.
-John
> On May 12, 2017, at 7:11 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 5/11/2017 12:02 PM, John Miller wrote:
>> Fixes: 9c7188a68d7b ("net/ark: provide API for hardware modules pktchkr and pktgen")
>> Coverity issue: 144513
>>
>> Fixes: 727b3fe292bc ("net/ark: integrate PMD")
>> Coverity issue: 144514
>>
>> Fixes: 9c7188a68d7b ("net/ark: provide API for hardware modules pktchkr and pktgen")
>> Coverity issue: 144512
>>
>> Fixes: 1131cbf0fb2b ("net/ark: stub PMD for Atomic Rules Arkville")
>> Coverity issue: 144517
>
> The convention is Coverity line first, Fixes line later.
>
>>
>> Fixes: 727b3fe292bc ("net/ark: integrate PMD")
>> Coverity issue: 144520
>
> Hi John,
>
> Thanks for fixing coverity issues.
>
> Can you please split patch into a patchset with multiple patches,
> grouped to same kind of fixes?
>
> And instead of having "coverity fix" in patch title, can you please
> describe what is really fixed, like "fix not null terminated buffer" or
> "fix missing function return check" etc ...
>
> Thanks,
> ferruh
>
>> Signed-off-by: John Miller <john.miller@atomicrules.com>
>
> <...>
>
>> --- a/drivers/net/ark/ark_pktgen.c
>> +++ b/drivers/net/ark/ark_pktgen.c
>> @@ -354,7 +354,8 @@ struct OPTIONS {
>> o->v.INT = atoll(val);
>> break;
>> case OTSTRING:
>> - strncpy(o->v.STR, val, ARK_MAX_STR_LEN);
>> + strncpy(o->v.STR, val, ARK_MAX_STR_LEN - 1);
>> + o->v.STR[ARK_MAX_STR_LEN - 1] = 0;
>
> This also works, but you can prefer to switch snprintf(), which
> guaranties the null termination.
>
>> break;
>> }
>> return 1;
>>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-05-13 11:28 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-11 11:02 [dpdk-dev] [PATCH] net/ark: fix for Coverity issues John Miller
2017-05-12 11:11 ` Ferruh Yigit
2017-05-13 11:28 ` john miller
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).