Skip to content

Commit

Permalink
Fix #649, use full object ID in timecb list
Browse files Browse the repository at this point in the history 
For the linked list of timer callbacks, use the full object ID
value rather than just the index.  This ensures consistency
in the list and also makes it easier to manage.
  • Loading branch information
@jphickey
jphickey committed Dec 2, 2020
1 parent 35925d1 commit 280695b
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 42 deletions.
4 changes: 2 additions & 2 deletions src/os/shared/inc/os-shared-time.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ typedef struct
char timer_name[OS_MAX_API_NAME];
uint32 flags;
OS_object_token_t timebase_token;
osal_index_t prev_ref;
osal_index_t next_ref;
osal_id_t prev_cb;
osal_id_t next_cb;
uint32 backlog_resets;
int32 wait_time;
int32 interval_time;
Expand Down
61 changes: 37 additions & 24 deletions src/os/shared/src/osapi-time.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,10 @@ static int32 OS_DoTimerAdd(osal_id_t *timer_id, const char *timer_name, osal_id_
osal_objtype_t objtype;
OS_object_token_t timebase_token;
OS_object_token_t timecb_token;
OS_object_token_t listcb_token;
OS_timecb_internal_record_t * timecb;
OS_timecb_internal_record_t * list_timecb;
OS_timebase_internal_record_t *timebase;
osal_id_t cb_list;
osal_index_t attach_id;

/*
** Check Parameters
Expand Down Expand Up @@ -166,27 +166,32 @@ static int32 OS_DoTimerAdd(osal_id_t *timer_id, const char *timer_name, osal_id_
timecb->callback_ptr = callback_ptr;
timecb->callback_arg = callback_arg;
timecb->flags = flags;
timecb->prev_ref = OS_ObjectIndexFromToken(&timecb_token);
timecb->next_ref = OS_ObjectIndexFromToken(&timecb_token);
timecb->prev_cb = OS_ObjectIdFromToken(&timecb_token);
timecb->next_cb = OS_ObjectIdFromToken(&timecb_token);

/*
* Now we need to add it to the time base callback ring, so take the
* timebase-specific lock to prevent a tick from being processed at this moment.
*/
OS_TimeBaseLock_Impl(&timebase_token);

cb_list = timebase->first_cb;
timebase->first_cb = OS_ObjectIdFromToken(&timecb_token);

if (OS_ObjectIdDefined(cb_list))
if (OS_ObjectIdGetById(OS_LOCK_MODE_NONE, OS_OBJECT_TYPE_OS_TIMECB, timebase->first_cb, &listcb_token) == OS_SUCCESS)
{
OS_ObjectIdToArrayIndex(OS_OBJECT_TYPE_OS_TIMECB, cb_list, &attach_id);
timecb->next_ref = attach_id;
timecb->prev_ref = OS_timecb_table[attach_id].prev_ref;
OS_timecb_table[timecb->prev_ref].next_ref = OS_ObjectIndexFromToken(&timecb_token);
OS_timecb_table[timecb->next_ref].prev_ref = OS_ObjectIndexFromToken(&timecb_token);
list_timecb = OS_OBJECT_TABLE_GET(OS_timecb_table, listcb_token);

timecb->next_cb = OS_ObjectIdFromToken(&listcb_token);
timecb->prev_cb = list_timecb->prev_cb;

if (OS_ObjectIdGetById(OS_LOCK_MODE_NONE, OS_OBJECT_TYPE_OS_TIMECB, timecb->prev_cb, &listcb_token) == OS_SUCCESS)
{
list_timecb->prev_cb = OS_ObjectIdFromToken(&timecb_token);
list_timecb = OS_OBJECT_TABLE_GET(OS_timecb_table, listcb_token);
list_timecb->next_cb = OS_ObjectIdFromToken(&timecb_token);
}
}

timebase->first_cb = OS_ObjectIdFromToken(&timecb_token);

OS_TimeBaseUnlock_Impl(&timebase_token);

/* Check result, finalize record, and unlock global table. */
Expand Down Expand Up @@ -399,8 +404,10 @@ int32 OS_TimerDelete(osal_id_t timer_id)
osal_id_t dedicated_timebase_id;
OS_object_token_t timecb_token;
OS_object_token_t timebase_token;
OS_object_token_t listcb_token;
OS_timebase_internal_record_t *timebase;
OS_timecb_internal_record_t * timecb;
OS_timecb_internal_record_t * list_timecb;

dedicated_timebase_id = OS_OBJECT_ID_UNDEFINED;
memset(&timebase_token, 0, sizeof(timebase_token));
Expand Down Expand Up @@ -436,25 +443,31 @@ int32 OS_TimerDelete(osal_id_t timer_id)
/*
* Now we need to remove it from the time base callback ring
*/
if (OS_ObjectIdEqual(timebase->first_cb, timer_id))
if (OS_ObjectIdEqual(timebase->first_cb, OS_ObjectIdFromToken(&timecb_token)))
{
if (timecb->next_ref != OS_ObjectIndexFromToken(&timecb_token))
if (OS_ObjectIdEqual(OS_ObjectIdFromToken(&timecb_token), timecb->next_cb))
{
OS_ObjectIdCompose_Impl(OS_OBJECT_TYPE_OS_TIMEBASE, timecb->next_ref, &timebase->first_cb);
timebase->first_cb = OS_OBJECT_ID_UNDEFINED;
}
else
{
/*
* consider the list empty
*/
timebase->first_cb = OS_OBJECT_ID_UNDEFINED;
timebase->first_cb = timecb->next_cb;
}
}

OS_timecb_table[timecb->prev_ref].next_ref = timecb->next_ref;
OS_timecb_table[timecb->next_ref].prev_ref = timecb->prev_ref;
timecb->next_ref = OS_ObjectIndexFromToken(&timecb_token);
timecb->prev_ref = OS_ObjectIndexFromToken(&timecb_token);
if(OS_ObjectIdGetById(OS_LOCK_MODE_NONE,OS_OBJECT_TYPE_OS_TIMECB,timecb->prev_cb,&listcb_token) == OS_SUCCESS)
{
list_timecb = OS_OBJECT_TABLE_GET(OS_timecb_table, listcb_token);
list_timecb->next_cb = timecb->next_cb;
}
if(OS_ObjectIdGetById(OS_LOCK_MODE_NONE,OS_OBJECT_TYPE_OS_TIMECB,timecb->next_cb,&listcb_token) == OS_SUCCESS)
{
list_timecb = OS_OBJECT_TABLE_GET(OS_timecb_table, listcb_token);
list_timecb->prev_cb = timecb->prev_cb;
}

timecb->next_cb = OS_ObjectIdFromToken(&timecb_token);
timecb->prev_cb = OS_ObjectIdFromToken(&timecb_token);

OS_TimeBaseUnlock_Impl(&timecb->timebase_token);

Expand Down
19 changes: 8 additions & 11 deletions src/os/shared/src/osapi-timebase.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -402,9 +402,7 @@ void OS_TimeBase_CallbackThread(osal_id_t timebase_id)
OS_timecb_internal_record_t * timecb;
OS_common_record_t * record;
OS_object_token_t token;
osal_index_t timer_id;
osal_index_t curr_cb_local_id;
osal_id_t curr_cb_public_id;
OS_object_token_t cb_token;
uint32 tick_time;
uint32 spin_cycles;
int32 saved_wait_time;
Expand Down Expand Up @@ -493,14 +491,12 @@ void OS_TimeBase_CallbackThread(osal_id_t timebase_id)
}

timebase->freerun_time += tick_time;
if (OS_ObjectIdToArrayIndex(OS_OBJECT_TYPE_OS_TIMECB, timebase->first_cb, &timer_id) == 0)
if (OS_ObjectIdGetById(OS_LOCK_MODE_NONE, OS_OBJECT_TYPE_OS_TIMECB, timebase->first_cb, &cb_token) == 0)
{
curr_cb_local_id = timer_id;
do
{
curr_cb_public_id = OS_global_timecb_table[curr_cb_local_id].active_id;
timecb = &OS_timecb_table[curr_cb_local_id];
saved_wait_time = timecb->wait_time;
timecb = OS_OBJECT_TABLE_GET(OS_timecb_table, cb_token);
saved_wait_time = timecb->wait_time;
timecb->wait_time -= tick_time;
while (timecb->wait_time <= 0)
{
Expand All @@ -525,7 +521,7 @@ void OS_TimeBase_CallbackThread(osal_id_t timebase_id)
*/
if (saved_wait_time > 0 && timecb->callback_ptr != NULL)
{
(*timecb->callback_ptr)(curr_cb_public_id, timecb->callback_arg);
(*timecb->callback_ptr)(OS_ObjectIdFromToken(&cb_token), timecb->callback_arg);
}

/*
Expand All @@ -536,8 +532,9 @@ void OS_TimeBase_CallbackThread(osal_id_t timebase_id)
break;
}
}
curr_cb_local_id = timecb->next_ref;
} while (curr_cb_local_id != timer_id);

} while (OS_ObjectIdGetById(OS_LOCK_MODE_NONE, OS_OBJECT_TYPE_OS_TIMECB, timecb->next_cb, &cb_token) == OS_SUCCESS &&
!OS_ObjectIdEqual(OS_ObjectIdFromToken(&cb_token), timebase->first_cb));
}

OS_TimeBaseUnlock_Impl(&token);
Expand Down
2 changes: 1 addition & 1 deletion src/tests/timer-add-api-test/timer-add-api-test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ void TestTimerAddApi(void)

OS_GetLocalTime(&EndTime);

for (i = 0; i < NUMBER_OF_TIMERS; i++)
for (i = NUMBER_OF_TIMERS-1; i >= 0; --i)
{
TimerStatus[i] = OS_TimerDelete(TimerID[i]);
}
Expand Down
4 changes: 2 additions & 2 deletions src/unit-test-coverage/shared/src/coveragetest-time.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,8 @@ void Test_OS_TimerDelete(void)
OS_timecb_table[1].timebase_token.obj_id = UT_OBJID_1;
OS_timecb_table[1].timebase_token.obj_idx = UT_INDEX_0;
OS_timecb_table[2].timebase_token = OS_timecb_table[1].timebase_token;
OS_timecb_table[2].next_ref = UT_INDEX_1;
OS_timecb_table[1].next_ref = UT_INDEX_1;
OS_timecb_table[2].next_cb = UT_OBJID_1;
OS_timecb_table[1].next_cb = UT_OBJID_1;
OS_timebase_table[0].first_cb = UT_OBJID_2;
actual = OS_TimerDelete(UT_OBJID_2);
UtAssert_True(actual == expected, "OS_TimerDelete() (%ld) == OS_SUCCESS", (long)actual);
Expand Down
9 changes: 7 additions & 2 deletions src/unit-test-coverage/shared/src/coveragetest-timebase.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,14 +243,19 @@ void Test_OS_TimeBase_CallbackThread(void)
* void OS_TimeBase_CallbackThread(uint32 timebase_id)
*/
OS_common_record_t *recptr;
OS_object_token_t timecb_token;

recptr = &OS_global_timebase_table[2];
memset(recptr, 0, sizeof(*recptr));
recptr->active_id = UT_OBJID_2;

OS_ObjectIdGetById(OS_LOCK_MODE_NONE, OS_OBJECT_TYPE_OS_TIMECB, UT_OBJID_1, &timecb_token);
OS_timebase_table[2].external_sync = UT_TimerSync;
OS_timecb_table[0].wait_time = 2000;
OS_timecb_table[0].callback_ptr = UT_TimeCB;
OS_timebase_table[2].first_cb = timecb_token.obj_id;
OS_timecb_table[1].prev_cb = timecb_token.obj_id;
OS_timecb_table[1].next_cb = timecb_token.obj_id;
OS_timecb_table[1].wait_time = 2000;
OS_timecb_table[1].callback_ptr = UT_TimeCB;
TimerSyncCount = 0;
TimerSyncRetVal = 0;
TimeCB = 0;
Expand Down

0 comments on commit 280695b

Please sign in to comment.