DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] examples: fix unchecked malloc return value in ip_pipeline
@ 2014-12-12 12:24 Bruce Richardson
  2014-12-12 15:19 ` Dumitrescu, Cristian
  0 siblings, 1 reply; 4+ messages in thread
From: Bruce Richardson @ 2014-12-12 12:24 UTC (permalink / raw)
  To: dev, cristian.dumitrescu

Static analysis shows that once instance of rte_zmalloc is missing
a return value check in the code. This is fixed by adding a return
value check. The malloc call itself is moved to earlier in the function
so that no work is done unless all memory allocation requests have
succeeded - thereby removing the need for rollback on error.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 examples/ip_pipeline/cmdline.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/examples/ip_pipeline/cmdline.c b/examples/ip_pipeline/cmdline.c
index 13d565e..152acb5 100644
--- a/examples/ip_pipeline/cmdline.c
+++ b/examples/ip_pipeline/cmdline.c
@@ -1093,7 +1093,7 @@ cmd_firewall_add_parsed(
 	__attribute__((unused)) void *data)
 {
 	struct cmd_firewall_add_result *params = parsed_result;
-	struct app_rule rule, *old_rule;
+	struct app_rule rule, *old_rule, *new_rule = NULL;
 	struct rte_mbuf *msg;
 	struct app_msg_req *req;
 	struct app_msg_resp *resp;
@@ -1148,6 +1148,18 @@ cmd_firewall_add_parsed(
 	if (msg == NULL)
 		rte_panic("Unable to allocate new message\n");
 
+	/* if we need a new rule structure, allocate it before we go further */
+	if (old_rule == NULL) {
+		new_rule = rte_zmalloc_socket("CLI", sizeof(struct app_rule),
+				RTE_CACHE_LINE_SIZE, rte_socket_id());
+		if (new_rule == NULL) {
+			printf("Cannot allocate memory for new rule\n");
+			rte_ctrlmbuf_free(msg);
+			return;
+		}
+	}
+
+
 	/* Fill request message */
 	req = (struct app_msg_req *)rte_ctrlmbuf_data(msg);
 	req->type = APP_MSG_REQ_FW_ADD;
@@ -1190,12 +1202,6 @@ cmd_firewall_add_parsed(
 		printf("Request FIREWALL_ADD failed (%u)\n", resp->result);
 	else {
 		if (old_rule == NULL) {
-			struct app_rule *new_rule = (struct app_rule *)
-				rte_zmalloc_socket("CLI",
-				sizeof(struct app_rule),
-				RTE_CACHE_LINE_SIZE,
-				rte_socket_id());
-
 			memcpy(new_rule, &rule, sizeof(rule));
 			TAILQ_INSERT_TAIL(&firewall_table, new_rule, entries);
 			n_firewall_rules++;
-- 
1.9.3

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] examples: fix unchecked malloc return value in ip_pipeline
  2014-12-12 12:24 [dpdk-dev] [PATCH] examples: fix unchecked malloc return value in ip_pipeline Bruce Richardson
@ 2014-12-12 15:19 ` Dumitrescu, Cristian
  2014-12-12 15:34   ` Thomas Monjalon
  2014-12-17  0:14   ` Thomas Monjalon
  0 siblings, 2 replies; 4+ messages in thread
From: Dumitrescu, Cristian @ 2014-12-12 15:19 UTC (permalink / raw)
  To: Richardson, Bruce, dev

Acked by: <Cristian.Dumitrescu at intel.com>

-----Original Message-----
From: Richardson, Bruce 
Sent: Friday, December 12, 2014 12:24 PM
To: dev@dpdk.org; Dumitrescu, Cristian
Cc: Richardson, Bruce
Subject: [PATCH] examples: fix unchecked malloc return value in ip_pipeline

Static analysis shows that once instance of rte_zmalloc is missing
a return value check in the code. This is fixed by adding a return
value check. The malloc call itself is moved to earlier in the function
so that no work is done unless all memory allocation requests have
succeeded - thereby removing the need for rollback on error.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 examples/ip_pipeline/cmdline.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/examples/ip_pipeline/cmdline.c b/examples/ip_pipeline/cmdline.c
index 13d565e..152acb5 100644
--- a/examples/ip_pipeline/cmdline.c
+++ b/examples/ip_pipeline/cmdline.c
@@ -1093,7 +1093,7 @@ cmd_firewall_add_parsed(
 	__attribute__((unused)) void *data)
 {
 	struct cmd_firewall_add_result *params = parsed_result;
-	struct app_rule rule, *old_rule;
+	struct app_rule rule, *old_rule, *new_rule = NULL;
 	struct rte_mbuf *msg;
 	struct app_msg_req *req;
 	struct app_msg_resp *resp;
@@ -1148,6 +1148,18 @@ cmd_firewall_add_parsed(
 	if (msg == NULL)
 		rte_panic("Unable to allocate new message\n");
 
+	/* if we need a new rule structure, allocate it before we go further */
+	if (old_rule == NULL) {
+		new_rule = rte_zmalloc_socket("CLI", sizeof(struct app_rule),
+				RTE_CACHE_LINE_SIZE, rte_socket_id());
+		if (new_rule == NULL) {
+			printf("Cannot allocate memory for new rule\n");
+			rte_ctrlmbuf_free(msg);
+			return;
+		}
+	}
+
+
 	/* Fill request message */
 	req = (struct app_msg_req *)rte_ctrlmbuf_data(msg);
 	req->type = APP_MSG_REQ_FW_ADD;
@@ -1190,12 +1202,6 @@ cmd_firewall_add_parsed(
 		printf("Request FIREWALL_ADD failed (%u)\n", resp->result);
 	else {
 		if (old_rule == NULL) {
-			struct app_rule *new_rule = (struct app_rule *)
-				rte_zmalloc_socket("CLI",
-				sizeof(struct app_rule),
-				RTE_CACHE_LINE_SIZE,
-				rte_socket_id());
-
 			memcpy(new_rule, &rule, sizeof(rule));
 			TAILQ_INSERT_TAIL(&firewall_table, new_rule, entries);
 			n_firewall_rules++;
-- 
1.9.3

--------------------------------------------------------------
Intel Shannon Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263
Business address: Dromore House, East Park, Shannon, Co. Clare

This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] examples: fix unchecked malloc return value in ip_pipeline
  2014-12-12 15:19 ` Dumitrescu, Cristian
@ 2014-12-12 15:34   ` Thomas Monjalon
  2014-12-17  0:14   ` Thomas Monjalon
  1 sibling, 0 replies; 4+ messages in thread
From: Thomas Monjalon @ 2014-12-12 15:34 UTC (permalink / raw)
  To: Dumitrescu, Cristian; +Cc: dev

Hi Cristian,

2014-12-12 15:19, Dumitrescu, Cristian:
> Acked by: <Cristian.Dumitrescu at intel.com>

Please, next time,
- add you acked-by below the signed-off,
- put your name and a real email address (like in a signed-off),
- and remove the patch to make email shorter.

I think the web site needs to be updated to explain such things.


> -----Original Message-----
> From: Richardson, Bruce 
> Sent: Friday, December 12, 2014 12:24 PM
> To: dev@dpdk.org; Dumitrescu, Cristian
> Cc: Richardson, Bruce
> Subject: [PATCH] examples: fix unchecked malloc return value in ip_pipeline
> 
> Static analysis shows that once instance of rte_zmalloc is missing
> a return value check in the code. This is fixed by adding a return
> value check. The malloc call itself is moved to earlier in the function
> so that no work is done unless all memory allocation requests have
> succeeded - thereby removing the need for rollback on error.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  examples/ip_pipeline/cmdline.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/examples/ip_pipeline/cmdline.c b/examples/ip_pipeline/cmdline.c
> index 13d565e..152acb5 100644
> --- a/examples/ip_pipeline/cmdline.c
> +++ b/examples/ip_pipeline/cmdline.c
[...]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] examples: fix unchecked malloc return value in ip_pipeline
  2014-12-12 15:19 ` Dumitrescu, Cristian
  2014-12-12 15:34   ` Thomas Monjalon
@ 2014-12-17  0:14   ` Thomas Monjalon
  1 sibling, 0 replies; 4+ messages in thread
From: Thomas Monjalon @ 2014-12-17  0:14 UTC (permalink / raw)
  To: Richardson, Bruce; +Cc: dev

> Static analysis shows that once instance of rte_zmalloc is missing
> a return value check in the code. This is fixed by adding a return
> value check. The malloc call itself is moved to earlier in the function
> so that no work is done unless all memory allocation requests have
> succeeded - thereby removing the need for rollback on error.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

> Acked by: <Cristian.Dumitrescu at intel.com>

Applied

Thanks
-- 
Thomas

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-12-17  0:14 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-12 12:24 [dpdk-dev] [PATCH] examples: fix unchecked malloc return value in ip_pipeline Bruce Richardson
2014-12-12 15:19 ` Dumitrescu, Cristian
2014-12-12 15:34   ` Thomas Monjalon
2014-12-17  0:14   ` Thomas Monjalon

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