Thread-safe rand for C

Question

I have this very short program for dealing with thread-safety of C's rand. Is it a correct implementation?

#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
typedef struct {
 pthread_mutex_t mutex;
} thread_safe_rand;
void thread_safe_rand_init(thread_safe_rand* tsr)
{
 if (tsr)
 {
 pthread_mutex_init(&tsr->mutex, NULL);
 }
}
void thread_safe_rand_free(thread_safe_rand* tsr)
{
 if (tsr)
 {
 pthread_mutex_destroy(&tsr->mutex);
 }
}
int thread_safe_rand_get(thread_safe_rand* tsr)
{
 int ret;
 if (tsr)
 {
 pthread_mutex_lock(&tsr->mutex);
 ret = rand();
 pthread_mutex_unlock(&tsr->mutex);
 return ret;
 }
 else
 {
 abort();
 }
}
int main() {
 srand(10);
 thread_safe_rand tsr;
 thread_safe_rand_init(&tsr);
 printf("%d\n", thread_safe_rand_get(&tsr));
 thread_safe_rand_free(&tsr);
}

Answer 1

Is it a correct implementation?

Some weaknesses:

Not C standard

Consider threads.h from the standard C library.

pthread_mutex...() and friends are not part of the C standard library, but I suppose the pthreads tag implies POSIX.

If you are using *nix, consider using a better random number functions than srand(), rand().

Lack of error checking

pthread_mutex_init(), pthread_mutex_destroy(), pthread_mutex_... return values deserve checking.

Initialize

thread_safe_rand tsr; deserve initialization since code flow does not look for errors.

Maybe thread_safe_rand tsr = { 0 };?

Always the same sequence

Perhaps this is just for testing purposes. srand(10);

---

Ref

Answer 2

Always scope your variables as tightly as possible. In thread_safe_rand_get your ret variable doesn't need to be scoped outside of the conditional. Nor do variables need to be declared before initialization (in anything approaching modern C).

int thread_safe_rand_get(thread_safe_rand* tsr)
{
 if (tsr)
 {
 pthread_mutex_lock(&tsr->mutex);
 int ret = rand();
 pthread_mutex_unlock(&tsr->mutex);
 return ret;
 }
 else
 {
 abort();
 }
}

This makes even more sense if you follow Toby Speight's advice and transform your function to have !tsr as a guard condition.

int thread_safe_rand_get(thread_safe_rand* tsr)
{
 if (!tsr)
 {
 abort();
 }
 pthread_mutex_lock(&tsr->mutex);
 int ret = rand();
 pthread_mutex_unlock(&tsr->mutex);
 return ret;
}

Answer 3

There's opportunity for early exit from thread_safe_rand_get():

 if (!tsr) {
 abort();
 }

That said, is it really our job to be testing whether the supplied pointer is null? I think we could just act like the standard library and say that it's the programmer's responsibility to pass a valid pointer.

We don't provide any testing to ensure that thread_safe_rand_init() and thread_safe_rand_free() are correctly paired up, for example, so we're not even consistent in our hand-holding.