[PATCHv3,2/5] validation: odp_timer.c: avoid dereferencing ptr after NULL check

Message ID 1422529273-31282-3-git-send-email-ola.liljedahl@linaro.org
State New
Headers show

Commit Message

Ola Liljedahl Jan. 29, 2015, 11:01 a.m.
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(-)

Comments

Anders Roxell Jan. 29, 2015, 2:11 p.m. | #1
On 2015-01-29 12:01, Ola Liljedahl 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 0238cf4..bc4fdf4 100644
> --- a/test/validation/odp_timer.c
> +++ b/test/validation/odp_timer.c
> @@ -69,23 +69,23 @@ static void handle_tmo(odp_event_t ev, bool stale, uint64_t prev_tick)
>  	if (ttp == NULL)
>  		CU_FAIL("odp_timeout_user_ptr() null user ptr");
>  
> -	if (ttp->ev2 != ev)
> +	if (ttp != NULL && ttp->ev2 != ev)
>  		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);

I think the change from printf to LOG_DBG should go into the cleanup
patch 4/5.

Cheers,
Anders
Ola Liljedahl Jan. 29, 2015, 3:31 p.m. | #2
On 29 January 2015 at 15:11, Anders Roxell <anders.roxell@linaro.org> wrote:
> On 2015-01-29 12:01, Ola Liljedahl 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 0238cf4..bc4fdf4 100644
>> --- a/test/validation/odp_timer.c
>> +++ b/test/validation/odp_timer.c
>> @@ -69,23 +69,23 @@ static void handle_tmo(odp_event_t ev, bool stale, uint64_t prev_tick)
>>       if (ttp == NULL)
>>               CU_FAIL("odp_timeout_user_ptr() null user ptr");
>>
>> -     if (ttp->ev2 != ev)
>> +     if (ttp != NULL && ttp->ev2 != ev)
>>               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);
>
> I think the change from printf to LOG_DBG should go into the cleanup
> patch 4/5.
OK

>
> Cheers,
> Anders

Patch

diff --git a/test/validation/odp_timer.c b/test/validation/odp_timer.c
index 0238cf4..bc4fdf4 100644
--- a/test/validation/odp_timer.c
+++ b/test/validation/odp_timer.c
@@ -69,23 +69,23 @@  static void handle_tmo(odp_event_t ev, bool stale, uint64_t prev_tick)
 	if (ttp == NULL)
 		CU_FAIL("odp_timeout_user_ptr() null user ptr");
 
-	if (ttp->ev2 != ev)
+	if (ttp != NULL && ttp->ev2 != ev)
 		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_event_t ev, bool stale, uint64_t prev_tick)
 		}
 	}
 
-	/* Use assert() for correctness check of test program itself */
-	assert(ttp->ev == ODP_EVENT_INVALID);
-	ttp->ev = ev;
+	if (ttp != NULL) {
+		/* Internal error */
+		CU_ASSERT_FATAL(ttp->ev == ODP_EVENT_INVALID);
+		ttp->ev = ev;
+	}
 }
 
 /* @private Worker thread entrypoint which performs timer alloc/set/cancel/free