| Message ID | 1465245630-31425-1-git-send-email-bill.fischofer@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 06/06/16 23:40, Bill Fischofer wrote: > Resolve Bug https://bugs.linaro.org/show_bug.cgi?id=2306 by cleaning up > TM queues and nodes in error paths. > > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> > --- > test/validation/traffic_mngr/traffic_mngr.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/test/validation/traffic_mngr/traffic_mngr.c b/test/validation/traffic_mngr/traffic_mngr.c > index a0cde43..208671b 100644 > --- a/test/validation/traffic_mngr/traffic_mngr.c > +++ b/test/validation/traffic_mngr/traffic_mngr.c > @@ -1325,12 +1325,19 @@ static int create_tm_queue(odp_tm_t odp_tm, > rc = odp_tm_queue_connect(tm_queue, tm_node); > if (rc != 0) { > LOG_ERR("odp_tm_queue_connect() failed\n"); > + odp_tm_queue_destroy(tm_queue); > return -1; > } > > return 0; > } > > +static int destroy_tm_queue(odp_tm_queue_t tm_queue) > +{ > + odp_tm_queue_disconnect(tm_queue); > + return odp_tm_queue_destroy(tm_queue); > +} > + > static tm_node_desc_t *create_tm_node(odp_tm_t odp_tm, > uint32_t level, > uint32_t num_levels, > @@ -1392,6 +1399,7 @@ static tm_node_desc_t *create_tm_node(odp_tm_t odp_tm, > if (rc != 0) { > LOG_ERR("odp_tm_node_connect() failed @ level=%u\n", > level); > + odp_tm_node_destroy(tm_node); > return NULL; > } > > @@ -1425,6 +1433,11 @@ static tm_node_desc_t *create_tm_node(odp_tm_t odp_tm, > if (rc != 0) { > LOG_ERR("create_tm_queue() failed @ level=%u\n", > level); > + while (priority > 0) > + destroy_tm_queue > + (queue_desc->tm_queues[--priority]); if you do not check return code function has to be void, not it. Interesting that checkpatch does not warn about splitting function name and ( on 2 lines. But if we did not have that it practice before it's better to not use it. To simplify reading I would move destroy path out of current loop, that also gave you more space for code. Maxim. > + free(queue_desc); > + free(node_desc); > return NULL; > } > }
On Thu, Jun 9, 2016 at 1:25 AM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 06/06/16 23:40, Bill Fischofer wrote: > >> Resolve Bug https://bugs.linaro.org/show_bug.cgi?id=2306 by cleaning up >> TM queues and nodes in error paths. >> >> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> >> --- >> test/validation/traffic_mngr/traffic_mngr.c | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/test/validation/traffic_mngr/traffic_mngr.c >> b/test/validation/traffic_mngr/traffic_mngr.c >> index a0cde43..208671b 100644 >> --- a/test/validation/traffic_mngr/traffic_mngr.c >> +++ b/test/validation/traffic_mngr/traffic_mngr.c >> @@ -1325,12 +1325,19 @@ static int create_tm_queue(odp_tm_t >> odp_tm, >> rc = odp_tm_queue_connect(tm_queue, tm_node); >> if (rc != 0) { >> LOG_ERR("odp_tm_queue_connect() failed\n"); >> + odp_tm_queue_destroy(tm_queue); >> return -1; >> } >> return 0; >> } >> +static int destroy_tm_queue(odp_tm_queue_t tm_queue) >> +{ >> + odp_tm_queue_disconnect(tm_queue); >> + return odp_tm_queue_destroy(tm_queue); >> +} >> + >> static tm_node_desc_t *create_tm_node(odp_tm_t odp_tm, >> uint32_t level, >> uint32_t num_levels, >> @@ -1392,6 +1399,7 @@ static tm_node_desc_t *create_tm_node(odp_tm_t >> odp_tm, >> if (rc != 0) { >> LOG_ERR("odp_tm_node_connect() failed @ level=%u\n", >> level); >> + odp_tm_node_destroy(tm_node); >> return NULL; >> } >> @@ -1425,6 +1433,11 @@ static tm_node_desc_t *create_tm_node(odp_tm_t >> odp_tm, >> if (rc != 0) { >> LOG_ERR("create_tm_queue() failed @ level=%u\n", >> level); >> + while (priority > 0) >> + destroy_tm_queue >> + >> (queue_desc->tm_queues[--priority]); >> > if you do not check return code function has to be void, not it. > > v2 sent to add the (void) as suggested. This call needs to be part of the loop since it needs to "unwind" any partial creates before freeing the queue and node backing storage. The split is unfortunate but the result of our line length rules that have precedence here. > Interesting that checkpatch does not warn about splitting function name > and ( on 2 lines. > But if we did not have that it practice before it's better to not use it. > To simplify reading I would move > destroy path out of current loop, that also gave you more space for code. > > Maxim. > > > + free(queue_desc); >> + free(node_desc); >> return NULL; >> } >> } >> > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp >
diff --git a/test/validation/traffic_mngr/traffic_mngr.c b/test/validation/traffic_mngr/traffic_mngr.c index a0cde43..208671b 100644 --- a/test/validation/traffic_mngr/traffic_mngr.c +++ b/test/validation/traffic_mngr/traffic_mngr.c @@ -1325,12 +1325,19 @@ static int create_tm_queue(odp_tm_t odp_tm, rc = odp_tm_queue_connect(tm_queue, tm_node); if (rc != 0) { LOG_ERR("odp_tm_queue_connect() failed\n"); + odp_tm_queue_destroy(tm_queue); return -1; } return 0; } +static int destroy_tm_queue(odp_tm_queue_t tm_queue) +{ + odp_tm_queue_disconnect(tm_queue); + return odp_tm_queue_destroy(tm_queue); +} + static tm_node_desc_t *create_tm_node(odp_tm_t odp_tm, uint32_t level, uint32_t num_levels, @@ -1392,6 +1399,7 @@ static tm_node_desc_t *create_tm_node(odp_tm_t odp_tm, if (rc != 0) { LOG_ERR("odp_tm_node_connect() failed @ level=%u\n", level); + odp_tm_node_destroy(tm_node); return NULL; } @@ -1425,6 +1433,11 @@ static tm_node_desc_t *create_tm_node(odp_tm_t odp_tm, if (rc != 0) { LOG_ERR("create_tm_queue() failed @ level=%u\n", level); + while (priority > 0) + destroy_tm_queue + (queue_desc->tm_queues[--priority]); + free(queue_desc); + free(node_desc); return NULL; } }
Resolve Bug https://bugs.linaro.org/show_bug.cgi?id=2306 by cleaning up TM queues and nodes in error paths. Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> --- test/validation/traffic_mngr/traffic_mngr.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)