| Message ID | 1421427959-11751-3-git-send-email-ola.liljedahl@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 16 January 2015 at 12:05, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: > Don't dereference pointer after successful check for NULL as this makes > Coverity > complain. (Coverity CID 85397, > https://bugs.linaro.org/show_bug.cgi?id=1056) > > Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> > --- > (This document/code contribution attached is provided under the terms of > agreement LES-LTM-21309) > > test/validation/odp_timer.c | 20 +++++++++++--------- > 1 file changed, 11 insertions(+), 9 deletions(-) > > diff --git a/test/validation/odp_timer.c b/test/validation/odp_timer.c > index dca01ce..5ba1b84 100644 > --- a/test/validation/odp_timer.c > +++ b/test/validation/odp_timer.c > @@ -69,23 +69,23 @@ static void handle_tmo(odp_buffer_t buf, bool stale, > uint64_t prev_tick) > if (ttp == NULL) > CU_FAIL("odp_timeout_user_ptr() null user ptr"); > If you use CU_FAIL_FATAL or CU_ASSERT_PTR_NOT_NULL_FATAL here, it will abort the test and you wont need to test for null in the following cases. http://cunit.sourceforge.net/doc/writing_tests.html#assertions > > - if (ttp->buf2 != buf) > + if (ttp != NULL && ttp->buf2 != buf) > CU_FAIL("odp_timeout_user_ptr() wrong user ptr"); > - if (ttp->tim != tim) > + if (ttp != NULL && ttp->tim != tim) > CU_FAIL("odp_timeout_timer() wrong timer"); > if (stale) { > if (odp_timeout_fresh(tmo)) > CU_FAIL("Wrong status (fresh) for stale timeout"); > /* Stale timeout => local timer must have invalid tick */ > - if (ttp->tick != TICK_INVALID) > + if (ttp != NULL && ttp->tick != TICK_INVALID) > CU_FAIL("Stale timeout for active timer"); > } else { > if (!odp_timeout_fresh(tmo)) > CU_FAIL("Wrong status (stale) for fresh timeout"); > /* Fresh timeout => local timer must have matching tick */ > - if (ttp->tick != tick) { > - printf("Wrong tick: expected %"PRIu64" actual > %"PRIu64"\n", > - ttp->tick, tick); > + if (ttp != NULL && ttp->tick != tick) { > + LOG_DBG("Wrong tick: expected %"PRIu64" actual > %"PRIu64"\n", > + ttp->tick, tick); > CU_FAIL("odp_timeout_tick() wrong tick"); > } > /* Check that timeout was delivered 'timely' */ > @@ -99,9 +99,11 @@ static void handle_tmo(odp_buffer_t buf, bool stale, > uint64_t prev_tick) > } > } > > - /* Use assert() for correctness check of test program itself */ > - assert(ttp->buf == ODP_BUFFER_INVALID); > - ttp->buf = buf; > + if (ttp != NULL) { > + /* Internal error */ > + CU_ASSERT_FATAL(ttp->buf == ODP_BUFFER_INVALID); > + ttp->buf = buf; > + } > } > > /* @private Worker thread entrypoint which performs timer > alloc/set/cancel/free > -- > 1.9.1 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
On 26 January 2015 at 14:48, Mike Holmes <mike.holmes@linaro.org> wrote: > > > On 16 January 2015 at 12:05, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: >> >> Don't dereference pointer after successful check for NULL as this makes >> Coverity >> complain. (Coverity CID 85397, >> https://bugs.linaro.org/show_bug.cgi?id=1056) >> >> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> >> --- >> (This document/code contribution attached is provided under the terms of >> agreement LES-LTM-21309) >> >> test/validation/odp_timer.c | 20 +++++++++++--------- >> 1 file changed, 11 insertions(+), 9 deletions(-) >> >> diff --git a/test/validation/odp_timer.c b/test/validation/odp_timer.c >> index dca01ce..5ba1b84 100644 >> --- a/test/validation/odp_timer.c >> +++ b/test/validation/odp_timer.c >> @@ -69,23 +69,23 @@ static void handle_tmo(odp_buffer_t buf, bool stale, >> uint64_t prev_tick) >> if (ttp == NULL) >> CU_FAIL("odp_timeout_user_ptr() null user ptr"); > > > If you use CU_FAIL_FATAL or CU_ASSERT_PTR_NOT_NULL_FATAL here, it will > abort the test and you wont need to test for null in the following cases. > http://cunit.sourceforge.net/doc/writing_tests.html#assertions Yes but this error is not a fatal error. I want the validation to continue even if the timer implementation returns a bogus user pointer. > >> >> >> - if (ttp->buf2 != buf) >> + if (ttp != NULL && ttp->buf2 != buf) >> CU_FAIL("odp_timeout_user_ptr() wrong user ptr"); >> - if (ttp->tim != tim) >> + if (ttp != NULL && ttp->tim != tim) >> CU_FAIL("odp_timeout_timer() wrong timer"); >> if (stale) { >> if (odp_timeout_fresh(tmo)) >> CU_FAIL("Wrong status (fresh) for stale timeout"); >> /* Stale timeout => local timer must have invalid tick */ >> - if (ttp->tick != TICK_INVALID) >> + if (ttp != NULL && ttp->tick != TICK_INVALID) >> CU_FAIL("Stale timeout for active timer"); >> } else { >> if (!odp_timeout_fresh(tmo)) >> CU_FAIL("Wrong status (stale) for fresh timeout"); >> /* Fresh timeout => local timer must have matching tick */ >> - if (ttp->tick != tick) { >> - printf("Wrong tick: expected %"PRIu64" actual >> %"PRIu64"\n", >> - ttp->tick, tick); >> + if (ttp != NULL && ttp->tick != tick) { >> + LOG_DBG("Wrong tick: expected %"PRIu64" actual >> %"PRIu64"\n", >> + ttp->tick, tick); >> CU_FAIL("odp_timeout_tick() wrong tick"); >> } >> /* Check that timeout was delivered 'timely' */ >> @@ -99,9 +99,11 @@ static void handle_tmo(odp_buffer_t buf, bool stale, >> uint64_t prev_tick) >> } >> } >> >> - /* Use assert() for correctness check of test program itself */ >> - assert(ttp->buf == ODP_BUFFER_INVALID); >> - ttp->buf = buf; >> + if (ttp != NULL) { >> + /* Internal error */ >> + CU_ASSERT_FATAL(ttp->buf == ODP_BUFFER_INVALID); >> + ttp->buf = buf; >> + } >> } >> >> /* @private Worker thread entrypoint which performs timer >> alloc/set/cancel/free >> -- >> 1.9.1 >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp > > > > > -- > Mike Holmes > Linaro Sr Technical Manager > LNG - ODP
On 16 January 2015 at 12:05, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: > Don't dereference pointer after successful check for NULL as this makes > Coverity > complain. (Coverity CID 85397, > https://bugs.linaro.org/show_bug.cgi?id=1056) > > Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> > Reviewed-by: Mike Holmes <mike.holmes@linaro.org> > --- > (This document/code contribution attached is provided under the terms of > agreement LES-LTM-21309) > > test/validation/odp_timer.c | 20 +++++++++++--------- > 1 file changed, 11 insertions(+), 9 deletions(-) > > diff --git a/test/validation/odp_timer.c b/test/validation/odp_timer.c > index dca01ce..5ba1b84 100644 > --- a/test/validation/odp_timer.c > +++ b/test/validation/odp_timer.c > @@ -69,23 +69,23 @@ static void handle_tmo(odp_buffer_t buf, bool stale, > uint64_t prev_tick) > if (ttp == NULL) > CU_FAIL("odp_timeout_user_ptr() null user ptr"); > > - if (ttp->buf2 != buf) > + if (ttp != NULL && ttp->buf2 != buf) > CU_FAIL("odp_timeout_user_ptr() wrong user ptr"); > - if (ttp->tim != tim) > + if (ttp != NULL && ttp->tim != tim) > CU_FAIL("odp_timeout_timer() wrong timer"); > if (stale) { > if (odp_timeout_fresh(tmo)) > CU_FAIL("Wrong status (fresh) for stale timeout"); > /* Stale timeout => local timer must have invalid tick */ > - if (ttp->tick != TICK_INVALID) > + if (ttp != NULL && ttp->tick != TICK_INVALID) > CU_FAIL("Stale timeout for active timer"); > } else { > if (!odp_timeout_fresh(tmo)) > CU_FAIL("Wrong status (stale) for fresh timeout"); > /* Fresh timeout => local timer must have matching tick */ > - if (ttp->tick != tick) { > - printf("Wrong tick: expected %"PRIu64" actual > %"PRIu64"\n", > - ttp->tick, tick); > + if (ttp != NULL && ttp->tick != tick) { > + LOG_DBG("Wrong tick: expected %"PRIu64" actual > %"PRIu64"\n", > + ttp->tick, tick); > CU_FAIL("odp_timeout_tick() wrong tick"); > } > /* Check that timeout was delivered 'timely' */ > @@ -99,9 +99,11 @@ static void handle_tmo(odp_buffer_t buf, bool stale, > uint64_t prev_tick) > } > } > > - /* Use assert() for correctness check of test program itself */ > - assert(ttp->buf == ODP_BUFFER_INVALID); > - ttp->buf = buf; > + if (ttp != NULL) { > + /* Internal error */ > + CU_ASSERT_FATAL(ttp->buf == ODP_BUFFER_INVALID); > + ttp->buf = buf; > + } > } > > /* @private Worker thread entrypoint which performs timer > alloc/set/cancel/free > -- > 1.9.1 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
diff --git a/test/validation/odp_timer.c b/test/validation/odp_timer.c index dca01ce..5ba1b84 100644 --- a/test/validation/odp_timer.c +++ b/test/validation/odp_timer.c @@ -69,23 +69,23 @@ static void handle_tmo(odp_buffer_t buf, bool stale, uint64_t prev_tick) if (ttp == NULL) CU_FAIL("odp_timeout_user_ptr() null user ptr"); - if (ttp->buf2 != buf) + if (ttp != NULL && ttp->buf2 != buf) CU_FAIL("odp_timeout_user_ptr() wrong user ptr"); - if (ttp->tim != tim) + if (ttp != NULL && ttp->tim != tim) CU_FAIL("odp_timeout_timer() wrong timer"); if (stale) { if (odp_timeout_fresh(tmo)) CU_FAIL("Wrong status (fresh) for stale timeout"); /* Stale timeout => local timer must have invalid tick */ - if (ttp->tick != TICK_INVALID) + if (ttp != NULL && ttp->tick != TICK_INVALID) CU_FAIL("Stale timeout for active timer"); } else { if (!odp_timeout_fresh(tmo)) CU_FAIL("Wrong status (stale) for fresh timeout"); /* Fresh timeout => local timer must have matching tick */ - if (ttp->tick != tick) { - printf("Wrong tick: expected %"PRIu64" actual %"PRIu64"\n", - ttp->tick, tick); + if (ttp != NULL && ttp->tick != tick) { + LOG_DBG("Wrong tick: expected %"PRIu64" actual %"PRIu64"\n", + ttp->tick, tick); CU_FAIL("odp_timeout_tick() wrong tick"); } /* Check that timeout was delivered 'timely' */ @@ -99,9 +99,11 @@ static void handle_tmo(odp_buffer_t buf, bool stale, uint64_t prev_tick) } } - /* Use assert() for correctness check of test program itself */ - assert(ttp->buf == ODP_BUFFER_INVALID); - ttp->buf = buf; + if (ttp != NULL) { + /* Internal error */ + CU_ASSERT_FATAL(ttp->buf == ODP_BUFFER_INVALID); + ttp->buf = buf; + } } /* @private Worker thread entrypoint which performs timer alloc/set/cancel/free
Don't dereference pointer after successful check for NULL as this makes Coverity complain. (Coverity CID 85397, https://bugs.linaro.org/show_bug.cgi?id=1056) Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> --- (This document/code contribution attached is provided under the terms of agreement LES-LTM-21309) test/validation/odp_timer.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)