Invalid use of void expression with threads (C)

Question

I've looked through tons of answers on here that skirt around my issue, but none answer the problem I am facing.

I am coding a multi-threaded program in C that does matrix multiplication to evaluate system performance.

Currently I am simply trying to start a single thread while passing in a couple variables.

This is my code

pthread_t *thread = (pthread_t *) malloc(sizeof(pthread_t));

int a = malloc(sizeof(int));
int b = malloc(sizeof(int));
a = 0;
b = size;
void **args = (void **) malloc(2 * sizeof(void *));
args[0] = (void *)a;
args[1] = (void *)b;

pthread_create(&thread[0], NULL, matrixMath, &args);

And the matrixMath method:

void* matrixMath(void* args) {
  int start = *((int *)args[0]);
  int end = *((int *)args[1]);

  printf("Start: %d, End: %d\n", start, end);

  return NULL;
}

Whenever I try to compile, I get an "invalid use of void expression" at both the "int start" and "int end" declarations. I based those lines on the discussion found here. Any help?

Answer 1

Since args is a void *, you can't dereference it, but args[0] does attempt to dereference it before the (int *) cast is applied. So, you need to reparenthesize so the void * is converted to an int * before you dereference it:

int start = ((int *)args)[0];
int end   = ((int *)args)[1];

Alternatively (but equivalently):

int *array = (int *)args;  // You could skip the cast in C
int start  = args[0];
int end    = args[1];

I'm not convinced about &args in the call to pthread_create() either. That passes a void *** to the thread code. I think you need something more like this, which has multiple merits including being simpler:

pthread_t thread;
int *a = malloc(2 * sizeof(int));
a[0] = 0;
a[1] = size;

pthread_create(&thread, NULL, matrixMath, a);

You could even use:

pthread_t thread;
int a[] = { 0, size };
pthread_create(&thread, NULL, matrixMath, a);

Answer 2

I'm going to demonstrate an alternative approach, using a struct. This alternative approach is easier to both code and maintain over time.

Your code has a few issues that might cause compiler and runtime errors, here are a few examples that should point you to the right path:

1. Your int assignment isn't performing what you might have wanted or intended it to perform.

   Your code assigns the numerical value of a pointer's address to an integer, probably truncating the data due to variable size differences... What you probably wanted was to store a pointer to an int.

   You wrote:

   int a = malloc(sizeof(int));
   int b = malloc(sizeof(int));
   

   You probably meant to write:

   int * a = malloc(sizeof(int));
   int * b = malloc(sizeof(int));
   

2. Your code is treating void * as a void ** (or int *) without using any casting...

   You wrote:

   int start = *((int *)args[0]);
   int end   = ((int *)args)[1];
   

   You probably meant to write:

   int start = ( (int *)args )[0];
   int end   = ( (int *)args )[1];
   

3. Your code performs malloc three (3) times, but never calls free (you have a memory leak).

It would be easier if you defined a struct to contain the data you wanted to "move" to (or share with) the thread.

For example, the following untested code probably doesn't work, but it clearly show the concept of how using a struct makes data transfer easier to manage, maintain and update. It also requires less calls to malloc, making memory management easier.

struct ThreadData {
   int start;
   int end;
};

void * thread_task(void *);

int main(void) {
   struct ThreadData * data = malloc(sizeof(*data));
   data->start = 0;
   data->end = 0; // = size;
   pthread_t thr;
   pthread_create(&thr, NULL, thread_task, data);
   // ... do whatever.
   // ... remember to join thread
}

void * thread_task(void * _data) {
   struct ThreadData * data = _data;
   printf("Start: %d, End: %d\n", data->start, data->end);
   // remember to free the memory when you're done.
   free(data);
   return NULL;
}

This approach is much easier to both code and maintain. Also, when you need to add data for the new thread, it's easy - just update the struct.

You can even put complex return values back into place-holders in the struct, allowing the thread to perform more complex tasks (remember to avoid having more than a single thread writing to the same struct field).