| Message ID | 1424765018-19634-1-git-send-email-maxim.uvarov@linaro.org |
|---|---|
| State | Accepted |
| Commit | 9c4a1435468e97a04f11ad4cedec34c54aaee715 |
| Headers | show |
Sure. Since we are checking the return value now for odp_pool_destroy() function goto statements can be removed. Reviewed-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> On Tuesday 24 February 2015 01:33 PM, Maxim Uvarov wrote: > From: Balasubramanian Manoharan <bala.manoharan@linaro.org> > > Fix for return value checking of odp_pool_destroy() function reported by Coverity. > https://bugs.linaro.org/show_bug.cgi?id=1142 > > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > --- > v2: Bala, that is your patch but I think it's reasonable to remove gotos. You > use them only once in function so just return there makes code simple. > What do you think? > > .../classification/odp_classification_tests.c | 17 ++++++++--------- > 1 file changed, 8 insertions(+), 9 deletions(-) > > diff --git a/test/validation/classification/odp_classification_tests.c b/test/validation/classification/odp_classification_tests.c > index 564455c..0537d99 100644 > --- a/test/validation/classification/odp_classification_tests.c > +++ b/test/validation/classification/odp_classification_tests.c > @@ -246,6 +246,7 @@ int classification_tests_init(void) > odp_queue_param_t qparam; > char queuename[ODP_QUEUE_NAME_LEN]; > int i; > + int ret; > > memset(¶m, 0, sizeof(param)); > param.pkt.seg_len = SHM_PKT_BUF_SIZE; > @@ -262,11 +263,15 @@ int classification_tests_init(void) > > pool_default = odp_pool_lookup("classification_pool"); > if (pool_default == ODP_POOL_INVALID) > - goto error_pool_default; > + return -1; > > pktio_loop = odp_pktio_open("loop", pool_default); > - if (pktio_loop == ODP_PKTIO_INVALID) > - goto error_pktio_loop; > + if (pktio_loop == ODP_PKTIO_INVALID) { > + ret = odp_pool_destroy(pool_default); > + if (ret) > + fprintf(stderr, "unable to destroy pool.\n"); > + return -1; > + } > qparam.sched.prio = ODP_SCHED_PRIO_DEFAULT; > qparam.sched.sync = ODP_SCHED_SYNC_ATOMIC; > qparam.sched.group = ODP_SCHED_GROUP_DEFAULT; > @@ -286,12 +291,6 @@ int classification_tests_init(void) > queue_list[i] = ODP_QUEUE_INVALID; > > return 0; > - > -error_pktio_loop: > - odp_pool_destroy(pool_default); > - > -error_pool_default: > - return -1; > } > > int classification_tests_finalize(void)
Merged. Maxim. On 02/24/2015 11:07 AM, Bala wrote: > Sure. Since we are checking the return value now for > odp_pool_destroy() function > goto statements can be removed. > > Reviewed-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> > On Tuesday 24 February 2015 01:33 PM, Maxim Uvarov wrote: >> From: Balasubramanian Manoharan <bala.manoharan@linaro.org> >> >> Fix for return value checking of odp_pool_destroy() function reported >> by Coverity. >> https://bugs.linaro.org/show_bug.cgi?id=1142 >> >> Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> >> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >> --- >> v2: Bala, that is your patch but I think it's reasonable to remove >> gotos. You >> use them only once in function so just return there makes code >> simple. >> What do you think? >> >> .../classification/odp_classification_tests.c | 17 >> ++++++++--------- >> 1 file changed, 8 insertions(+), 9 deletions(-) >> >> diff --git >> a/test/validation/classification/odp_classification_tests.c >> b/test/validation/classification/odp_classification_tests.c >> index 564455c..0537d99 100644 >> --- a/test/validation/classification/odp_classification_tests.c >> +++ b/test/validation/classification/odp_classification_tests.c >> @@ -246,6 +246,7 @@ int classification_tests_init(void) >> odp_queue_param_t qparam; >> char queuename[ODP_QUEUE_NAME_LEN]; >> int i; >> + int ret; >> memset(¶m, 0, sizeof(param)); >> param.pkt.seg_len = SHM_PKT_BUF_SIZE; >> @@ -262,11 +263,15 @@ int classification_tests_init(void) >> pool_default = odp_pool_lookup("classification_pool"); >> if (pool_default == ODP_POOL_INVALID) >> - goto error_pool_default; >> + return -1; >> pktio_loop = odp_pktio_open("loop", pool_default); >> - if (pktio_loop == ODP_PKTIO_INVALID) >> - goto error_pktio_loop; >> + if (pktio_loop == ODP_PKTIO_INVALID) { >> + ret = odp_pool_destroy(pool_default); >> + if (ret) >> + fprintf(stderr, "unable to destroy pool.\n"); >> + return -1; >> + } >> qparam.sched.prio = ODP_SCHED_PRIO_DEFAULT; >> qparam.sched.sync = ODP_SCHED_SYNC_ATOMIC; >> qparam.sched.group = ODP_SCHED_GROUP_DEFAULT; >> @@ -286,12 +291,6 @@ int classification_tests_init(void) >> queue_list[i] = ODP_QUEUE_INVALID; >> return 0; >> - >> -error_pktio_loop: >> - odp_pool_destroy(pool_default); >> - >> -error_pool_default: >> - return -1; >> } >> int classification_tests_finalize(void) >
diff --git a/test/validation/classification/odp_classification_tests.c b/test/validation/classification/odp_classification_tests.c index 564455c..0537d99 100644 --- a/test/validation/classification/odp_classification_tests.c +++ b/test/validation/classification/odp_classification_tests.c @@ -246,6 +246,7 @@ int classification_tests_init(void) odp_queue_param_t qparam; char queuename[ODP_QUEUE_NAME_LEN]; int i; + int ret; memset(¶m, 0, sizeof(param)); param.pkt.seg_len = SHM_PKT_BUF_SIZE; @@ -262,11 +263,15 @@ int classification_tests_init(void) pool_default = odp_pool_lookup("classification_pool"); if (pool_default == ODP_POOL_INVALID) - goto error_pool_default; + return -1; pktio_loop = odp_pktio_open("loop", pool_default); - if (pktio_loop == ODP_PKTIO_INVALID) - goto error_pktio_loop; + if (pktio_loop == ODP_PKTIO_INVALID) { + ret = odp_pool_destroy(pool_default); + if (ret) + fprintf(stderr, "unable to destroy pool.\n"); + return -1; + } qparam.sched.prio = ODP_SCHED_PRIO_DEFAULT; qparam.sched.sync = ODP_SCHED_SYNC_ATOMIC; qparam.sched.group = ODP_SCHED_GROUP_DEFAULT; @@ -286,12 +291,6 @@ int classification_tests_init(void) queue_list[i] = ODP_QUEUE_INVALID; return 0; - -error_pktio_loop: - odp_pool_destroy(pool_default); - -error_pool_default: - return -1; } int classification_tests_finalize(void)