hartmannathan commented on code in PR #3025: URL: https://github.com/apache/nuttx-apps/pull/3025#discussion_r1992142071
########## testing/ostest/mutex.c: ########## @@ -103,11 +116,172 @@ static void *thread_func(FAR void *parameter) return NULL; /* Non-reachable -- needed for some compilers */ } +static void *moved_mutex_thread_func(FAR void *parameter) +{ + FAR mutex_thread_args_t *args = (FAR mutex_thread_args_t *)parameter; + + int id = args->thread_id; + int ndx = id - 1; + int i; + unsigned long local_loops = 0; + + for (local_loops = 0; local_loops < NLOOPS; local_loops++) + { + int status = pthread_mutex_lock(args->mutex_ptr); + if (status != 0) + { + printf("ERROR moved mutex thread %d: pthread_mutex_lock failed, " + "status=%d\n", id, status); + ASSERT(false); + } + + if (*(args->mutex_value) == 1) + { + printf("ERROR moved mutex thread=%d: " + "moved_mutex_value should be zero, " + "instead moved_mutex_value=%d\n", + id, *(args->mutex_value)); + ASSERT(false); + args->nerrors[ndx]++; + } + + *(args->mutex_value) = 1; + for (i = 0; i < 10; i++) Review Comment: Above, in the outer loop, we are using NLOOPS define; should there be a similar define for this loop, e.g., `NYIELDS`? ########## testing/ostest/mutex.c: ########## @@ -103,11 +116,172 @@ static void *thread_func(FAR void *parameter) return NULL; /* Non-reachable -- needed for some compilers */ } +static void *moved_mutex_thread_func(FAR void *parameter) +{ + FAR mutex_thread_args_t *args = (FAR mutex_thread_args_t *)parameter; + + int id = args->thread_id; + int ndx = id - 1; + int i; + unsigned long local_loops = 0; Review Comment: Something doesn't seem right with this suggestion. Did you mean: ```suggestion int local_loops; ``` ? ########## testing/ostest/mutex.c: ########## @@ -103,11 +116,172 @@ static void *thread_func(FAR void *parameter) return NULL; /* Non-reachable -- needed for some compilers */ } +static void *moved_mutex_thread_func(FAR void *parameter) +{ + FAR mutex_thread_args_t *args = (FAR mutex_thread_args_t *)parameter; + + int id = args->thread_id; + int ndx = id - 1; + int i; + unsigned long local_loops = 0; + + for (local_loops = 0; local_loops < NLOOPS; local_loops++) + { + int status = pthread_mutex_lock(args->mutex_ptr); + if (status != 0) + { + printf("ERROR moved mutex thread %d: pthread_mutex_lock failed, " + "status=%d\n", id, status); + ASSERT(false); + } + + if (*(args->mutex_value) == 1) + { + printf("ERROR moved mutex thread=%d: " + "moved_mutex_value should be zero, " + "instead moved_mutex_value=%d\n", + id, *(args->mutex_value)); + ASSERT(false); + args->nerrors[ndx]++; + } + + *(args->mutex_value) = 1; + for (i = 0; i < 10; i++) + { + pthread_yield(); + } + + *(args->mutex_value) = 0; + + status = pthread_mutex_unlock(args->mutex_ptr); + if (status != 0) + { + printf("ERROR moved mutex thread %d: pthread_mutex_unlock failed, " + "status=%d\n", id, status); + ASSERT(false); + } + } + + args->nloops[ndx] = local_loops; + pthread_exit(NULL); + return NULL; +} + /**************************************************************************** - * Public Functions + * Name: mutex_move_test + * + * Description: + * Test the mutex move functionality. This test creates a mutex, moves + * it to a new location, and then starts two threads that use the moved + * mutex. + * POSIX specification does not define the behavior of a mutex that is + * moved. This test is intended to verify that the mutex can be moved, + * it's useful for some cases, see the discussion in Review Comment: ```suggestion * which is useful for some cases, see the discussion in ``` ########## testing/ostest/mutex.c: ########## @@ -103,11 +116,172 @@ static void *thread_func(FAR void *parameter) return NULL; /* Non-reachable -- needed for some compilers */ } +static void *moved_mutex_thread_func(FAR void *parameter) +{ + FAR mutex_thread_args_t *args = (FAR mutex_thread_args_t *)parameter; + + int id = args->thread_id; + int ndx = id - 1; + int i; + unsigned long local_loops = 0; Review Comment: By the way, if changing `local_loops` to `int` here, then `nloops` of `mutex_thread_args_t` needs to be `int` also? Or is it better to leave as unsigned long? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org