Dynamically allocating threads in C

Question

I'm creating a little program in C which calculates the faculty of numbers the user enters, until the user enters a negative number. It does this using threads.

I get a segmentation fault when running so I'm doing something wrong, but I don't know what exactly.

Here is my code:

/* 
 * File:   main.c
 * Author: thomasvanhelden
 *
 * Created on June 15, 2014, 3:17 AM
 */

#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>

/**
 * Calculates faculty of given number
 *      Can run as a thread
 * @param param The given number
 * @return Doesn't return anything
 */
void *fac(void *param) {
    int* number = (int*) param;
    int faculty = 1;
    int i; // counter

    printf("De faculteit van %i is: ", number);

    for (i = 2; i <= number; i++) {
        faculty *= i;
    }

    printf("%i\n", faculty);
}

/*
 * 
 */
int main(int argc, char** argv) {
    pthread_t **threads; 
    int getal, numThreads = 0, counter, stop = 0;

    // ask for numbers until user enters negative number
    while (stop == 0) {
        scanf("%i", getal);

        if (getal >= 0) {
            numThreads++;
            threads = realloc(threads, sizeof(pthread_t*) * (numThreads+1));
            threads[numThreads - 1] = malloc(sizeof(pthread_t));

            // Create the thread
            if (pthread_create(&threads[numThreads - 1], NULL, fac, &getal)) {
                // something went wrong
                printf("Error creating thread %i!\n", numThreads);
                return 1;
            }

        } else {
            // User entered negative number and wants to stop
            stop = 1;
        }
    }

    // join all the threads
    for (counter = 0; counter < numThreads; counter++) {
        if (pthread_join(threads[counter], NULL)) {
            printf("Something went wrong joining thread %i\n", counter + 1);
        }
    }


    return (EXIT_SUCCESS);
}

Answer 1

I think you mean "factorial"? Anyway, there are multiple problems with your code. Even without strict flags enabled, I get the following warnings:

fac.c:23:40: warning: format specifies type 'int' but the argument has type 'int *' [-Wformat]
    printf("De faculteit van %i is: ", number);
                             ~~        ^~~~~~
fac.c:25:19: warning: ordered comparison between pointer and integer ('int' and 'int *')
    for (i = 2; i <= number; i++) {
                ~ ^  ~~~~~~
fac.c:30:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
fac.c:41:21: warning: format specifies type 'int *' but the argument has type 'int' [-Wformat]
        scanf("%i", getal);
               ~~   ^~~~~
fac.c:49:32: warning: incompatible pointer types passing 'pthread_t **' (aka 'struct _opaque_pthread_t ***') to
      parameter of type 'pthread_t *' (aka 'struct _opaque_pthread_t **'); remove &
      [-Wincompatible-pointer-types]
            if (pthread_create(&threads[numThreads - 1], NULL, fac, &getal)) {
                               ^~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/pthread.h:310:42: note: passing argument to parameter here
int pthread_create(pthread_t * __restrict, const pthread_attr_t * __restrict,
                                         ^
fac.c:63:26: warning: incompatible pointer types passing 'pthread_t *' (aka 'struct _opaque_pthread_t **') to
      parameter of type 'pthread_t' (aka 'struct _opaque_pthread_t *'); dereference with *
      [-Wincompatible-pointer-types]
        if (pthread_join(threads[counter], NULL)) {
                         ^~~~~~~~~~~~~~~~
                         *
/usr/include/pthread.h:333:28: note: passing argument to parameter here
int pthread_join(pthread_t , void **) __DARWIN_ALIAS_C(pthread_join);
                           ^
6 warnings generated.

As it is, if you run the program under gdb, it crashes after you enter a number:

...
4

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000
0x00007fff92c1401a in __svfscanf_l ()
(gdb) bt
#0  0x00007fff92c1401a in __svfscanf_l ()
#1  0x00007fff92c0c0eb in scanf ()
#2  0x0000000100000d72 in main (argc=1, argv=0x7fff5fbffa18) at fac.c:41

Looking at the stack trace with bt, we see immediately that it is crashing at line 41 where you call scanf(). And if you look back at the compiler warnings, it's telling you that it expects an int* but you're only passing an int. This is because scanf needs a pointer in order to modify the variable with the user input.

If you fix all the remaining warnings, your code works.

• You need to dereference number in the fac function when you want its value
• You need to initialise threads to NULL so realloc will work (it is otherwise undefined, presumably non-zero and thus the buffer will never be (re)allocated
• The pthread_create and pthread_join calls need updating so the value/pointer matches the expected signature

After making the changes above, I can run it as intended:

$ ./fac_fixed
3
De faculteit van 3 is: 6
4
De faculteit van 4 is: 24
5
De faculteit van 5 is: 120
6
De faculteit van 6 is: 720
7
De faculteit van 7 is: 5040
8
De faculteit van 8 is: 40320
9
De faculteit van 9 is: 362880
12
De faculteit van 12 is: 479001600
-1

As to the structure of the code, I don't see why you want to create an array of threads anyway, as you are only calculating one at a time, sequentially. You only really need at most one worker thread, but since main is doing nothing but blocking on a join anyway, it doesn't really need to be in a thread. But at least it doesn't crash!

Answer 2

When you call realloc() the first time, you should have threads set to NULL. As it is, you are passing an uninitalized pointer which is undefined behaviour.

1) Initialize it to 0:

pthread_t **threads = 0; 

2) You are also printing the pointers rather than what they point to here:

printf("De faculteit van %i is: ", *number);

3) scanf() needs & as it expects int * for %d.

scanf("%i", &getal);