From 62bb375688f215aea630095416df8dd8a169d295 Mon Sep 17 00:00:00 2001 From: Michael Lotz Date: Tue, 6 Dec 2011 02:30:17 +0100 Subject: [PATCH] Restructure wait_for_thread_etc() to make it easier to follow. * Avoid needless adding of the death entry if the sem is gone already. * Delete objects as soon as they aren't needed anymore and return early where possible. * Contain the thread == NULL case in its block and return from there as well instead of non-obviously figuring out what happened later. * Pull out the return code asignment. * Minor cleanup. --- src/system/kernel/thread.cpp | 46 +++++++++++++----------------------- 1 file changed, 17 insertions(+), 29 deletions(-) diff --git a/src/system/kernel/thread.cpp b/src/system/kernel/thread.cpp index 43e69d3350..e6567a1347 100644 --- a/src/system/kernel/thread.cpp +++ b/src/system/kernel/thread.cpp @@ -2465,10 +2465,7 @@ status_t wait_for_thread_etc(thread_id id, uint32 flags, bigtime_t timeout, status_t *_returnCode) { - job_control_entry* freeDeath = NULL; - status_t status = B_OK; - - if (id < B_OK) + if (id < 0) return B_BAD_THREAD_ID; // get the thread, queue our death entry, and fetch the semaphore we have to @@ -2480,16 +2477,14 @@ wait_for_thread_etc(thread_id id, uint32 flags, bigtime_t timeout, if (thread != NULL) { // remember the semaphore we have to wait on and place our death entry exitSem = thread->exit.sem; - list_add_link_to_head(&thread->exit.waiters, &death); + if (exitSem >= 0) + list_add_link_to_head(&thread->exit.waiters, &death); thread->UnlockAndReleaseReference(); - // Note: We mustn't dereference the pointer afterwards, only check - // it. - } - thread_death_entry* threadDeathEntry = NULL; - - if (thread == NULL) { + if (exitSem < 0) + return B_BAD_THREAD_ID; + } else { // we couldn't find this thread -- maybe it's already gone, and we'll // find its death entry in our team Team* team = thread_get_current_thread()->team; @@ -2498,47 +2493,43 @@ wait_for_thread_etc(thread_id id, uint32 flags, bigtime_t timeout, // check the child death entries first (i.e. main threads of child // teams) bool deleteEntry; - freeDeath = team_get_death_entry(team, id, &deleteEntry); + job_control_entry* freeDeath + = team_get_death_entry(team, id, &deleteEntry); if (freeDeath != NULL) { death.status = freeDeath->status; - if (!deleteEntry) - freeDeath = NULL; + if (deleteEntry) + delete freeDeath; } else { // check the thread death entries of the team (non-main threads) + thread_death_entry* threadDeathEntry = NULL; while ((threadDeathEntry = (thread_death_entry*)list_get_next_item( &team->dead_threads, threadDeathEntry)) != NULL) { if (threadDeathEntry->thread == id) { list_remove_item(&team->dead_threads, threadDeathEntry); team->dead_threads_count--; death.status = threadDeathEntry->status; + free(threadDeathEntry); break; } } if (threadDeathEntry == NULL) - status = B_BAD_THREAD_ID; + return B_BAD_THREAD_ID; } - } - if (thread == NULL && status == B_OK) { // we found the thread's death entry in our team if (_returnCode) *_returnCode = death.status; - delete freeDeath; - free(threadDeathEntry); return B_OK; } // we need to wait for the death of the thread - if (exitSem < 0) - return B_BAD_THREAD_ID; - resume_thread(id); // make sure we don't wait forever on a suspended thread - status = acquire_sem_etc(exitSem, 1, flags, timeout); + status_t status = acquire_sem_etc(exitSem, 1, flags, timeout); if (status == B_OK) { // this should never happen as the thread deletes the semaphore on exit @@ -2546,9 +2537,6 @@ wait_for_thread_etc(thread_id id, uint32 flags, bigtime_t timeout, } else if (status == B_BAD_SEM_ID) { // this is the way the thread normally exits status = B_OK; - - if (_returnCode) - *_returnCode = death.status; } else { // We were probably interrupted or the timeout occurred; we need to // remove our death entry now. @@ -2563,12 +2551,12 @@ wait_for_thread_etc(thread_id id, uint32 flags, bigtime_t timeout, // middle of the cleanup. acquire_sem(exitSem); status = B_OK; - - if (_returnCode != NULL) - *_returnCode = death.status; } } + if (status == B_OK && _returnCode != NULL) + *_returnCode = death.status; + return status; }