Search code examples
ccastingpthreadsargv

How do I pass an integer into the pthread_create function from argv? (C)


For this program I pass in numbers through the command line and then have a multithreaded program that takes each argument, calculates its factors, then prints them. I know c++, but I'm rough with c and can't seem to get the casting down correctly for this program. Specifically when I pass the argument into the thread_create and cast it to an integer. The code I have below compiles, but the value after the cast is always a 0. How do I cast the char value to a void* then to an integer?

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

    #define MAX_ARRAY (17)

    void *thread_func(void *);

    int factors[MAX_ARRAY];

    int main(int argc, char *argv[]) {

            pthread_t thread_handle[argc];
            int i;
            int g;

            // Create Children Threads
            for ( i = 0; i < argc; i++ ) {
                    pthread_create(&thread_handle[i], NULL, thread_func, &argv[i + 1]);
            }

            // Rejoin Threads
            for ( i = 0; i < argc; i++ ) {
                    pthread_join(thread_handle[i], NULL);

                    // Print Factors Here
                    printf("%d: ", atoi(argv[i]));
                    for ( g = 0; g < MAX_ARRAY; g++ ) {
                            printf("%d, ", factors[g]);
                    }

                    printf("\n");

                    for ( g = 0; g < MAX_ARRAY; g++ ) {
                            factors[g] = 0;
                    }
            }
            return 0;
    }

    void *thread_func(void *data) {
            int n = atoi(data); 
            int x;
            int v;

            printf("Number to factor is: %d\n", n);

            for ( x = 1; x <= n; ++x ) {
                    if (n%x == 0)
                            factors[v++] = x;
            }
            return NULL;
    }

Solution

  • The problem is that each thread uses the same array for the factors, without any synchronization. But if each thread had to get a lock for the array before running, they would in effect all run in sequence which would defeat the purpose of threading.

    Incidentally, argv[0] is the program name, which you should skip.

    What you should do is have a different factor array for each thread, so that they can work independently without interference. You should also do all the display in the main thread, to control the order in which things are printed.

    Since it is probably best to display the factors in order, you should first create all the threads, then join all of them, then finally display the results.

    There were also a few small errors here and there like off by one errors or uninitialized variables.

    Here is a corrected version:

    #include <stdio.h>
    #include <stdlib.h>
    #include <pthread.h>
    
    #define MAX_ARRAY   17
    
    typedef struct {
        int factors[MAX_ARRAY];
        int n;
    } thread_data;
    
    void * thread_func (void *);
    
    int main (int argc, char *argv[]) {
        int n = argc - 1;
        pthread_t thread_handle[n];
        thread_data thread_data_table[n];
        int i;
    
        // Create Children Threads
        for (i = 0; i < n; i++ ) {
            thread_data_table[i].n = atoi (argv[i + 1]);
            pthread_create(&thread_handle[i], NULL, thread_func,
                           &thread_data_table[i]);
        }
    
        // Join Threads
        for (i = 0; i < n; i++ ) {
            pthread_join(thread_handle[i], NULL);
        }
    
        // Print Factors
        for (i = 0; i < n; i++) {
            int j;
    
            printf("%d: ", thread_data_table[i].n);
            for (j = 0; j < MAX_ARRAY; j++ ) {
                int x = thread_data_table[i].factors[j];
                if (x == 0) {
                    break;
                }
                printf("%d, ", x);
            }
            printf("\n");        
        }
        return 0;
    }
    
    void * thread_func (void *data)
    {
        thread_data *p = (thread_data*)data;
        int i;
        int count = 0;
    
        for (i = 1; i <= p->n; ++i ) {
            if (p->n % i == 0) {
                if (count == MAX_ARRAY) {
                    break;
                }
                p->factors[count++] = i;
            }
        }
        if (count < MAX_ARRAY) {
            p->factors[count] = 0;
        }
    
        return NULL;
    }