Pthread Posix prime factorization getting weird results?

Question

I have this project where I recieve input from the command line such as "54 342 12" and it is suppose to make a thread for each input and have the threads return an array of integers and then the main thread is supppose to print out the different prime factorizations. however I am recieving weird output such as a bunch of zeros. I have no idea why and any help would be greatly appreciated.

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


typedef struct _thread_data_t {
        int tid;
        } thread_data_t;

void *runner(void *param);

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

        pthread_t thr[argc];
        pthread_attr_t attr;
        int i, rc;
        //int *primeFactor;
        //primeFactor = (int *)malloc(sizeof(int)*argc);
        //thread_data_t thr_data[argc];
        printf("Prime Numbers: ");

        //Get the default attributes
        pthread_attr_init(&attr);
        //creat the thread
        for(i = 1; i < argc; ++i){
        //thr_data[i].tid = i;
        if ((rc = pthread_create(&thr[i],&attr,runner,argv[i]))){
                fprintf(stderr, "error: pthread_create, rc: %d\n", rc);
                return EXIT_FAILURE;
                }
        }

        //Wait for the thread to exit
        for(i = 1; i < argc; ++i){
        void *returnValue;
        int r = 0;
        int x = (sizeof(returnValue) / sizeof(returnValue[0])) - 1;
        pthread_join(thr[i], &returnValue);
                for(r = 0; r < x; r++){
                //int c = (int *)returnValue[r];
                printf("%d ",  ((int *)returnValue)[r]);
                }
        }
        printf("\nComplete\n");

}

//The Thread will begin control in this function
void *runner(void *param) {
        int *primeFactors;
        int num = atoi(param);
        primeFactors = (int *)malloc(sizeof(int)*num);
        int i, j, isPrime;
        int k = 0;
        for(i=2; i<=num; i++)
        {
                if(num%i==0)
                {
                        isPrime=1;
                        for(j=2; j<=i/2; j++)
                        {
                                if(i%j==0)
                                {
                                        isPrime = 0;
                                        break;
                                }
                        }

                        if(isPrime==1)
                        {
                                primeFactors[k] = i;
                                k++;
                        }
                }
        }


        //Exit the thread
//      pthread_exit(0);

//      pthread_exit((void *)primeFactors);
        pthread_exit(primeFactors);
}

Answer 1

This line is a problem:

int x = (sizeof(returnValue) / sizeof(returnValue[0])) - 1;

The sizeof array / sizeof array[0] works only with pure arrays, not with pointers. Note that returnValue is a pointer, so sizeof(returnValue) does not return the size in bytes of the sequence of ints that the thread created, it gives you the number of bytes that a pointer needs to be store in memory. On x86_64 architecture it will be most likely 8, so x will be in most cases larger than the actuall number of prime factors and you would access the pointer out of bound which is why you see garbage values.

Because you are returning a pointer to a malloced location, you need to return the length as well. The best way to do it is by creating a structure for the return value and store the information there.

Create a struct

struct thread_result
{
    int *factors;
    size_t len;
};

and return a pointer to this struct with the information:

void *runner(void *param) {
        int *primeFactors;
        int num = atoi(param);
        if(num == 0)
            pthread_exit(NULL);

        struct thread_result *res = calloc(1, sizeof *res);

        res->factors = NULL;
        res->len = 0;

        int *tmp;

        if(res == NULL)
            pthread_exit(NULL);

        int i, j, isPrime;
        int k = 0;
        for(i=2; i<=num; i++)
        {
            if(num%i==0)
            {
                isPrime=1;
                for(j=2; j<=i/2; j++)
                {
                    if(i%j==0)
                    {
                            isPrime = 0;
                            break;
                    }
                }

                if(isPrime==1)
                {
                    tmp = realloc(res->factors, (k+1) * sizeof *res->factors);
                    if(tmp == NULL)
                    {
                        free(res->factors);
                        free(res);
                        pthread_exit(NULL);
                    }
                    res->factors = tmp;

                    res->factors[k++] = i;
                    res->len = k;
                }
            }
        }


        pthread_exit(res);
}

Now you can get the values like this:

for(i = 1; i < argc; ++i){
    void *data;
    pthread_join(thr[i], &data);

    if(data == NULL)
        continue; // error in thread

    struct thread_result *res = data;

    for(size_t r = 0; r < res->len; r++){
        printf("%d ", res->factors[r]);
    }

    free(res->factors);
    free(res);
}

And don't forget to destroy the thread attributes with

pthread_attr_destroy(&attr);

before leaving main. But you are not setting any attribute to the threads, so you can creates the threads with:

pthread_create(&thr[i], NULL, runner, argv[i]);

Also don't cast malloc and you have to check the return value of malloc.

edit

OP wrote in the comments

I think I updated all of my code correctly to match what you said but now I am recieving the error "Segmentation dump (core dumped)" Any idea on why this might be?

Actually no, but you must have missed something somewhere, because when I compile and run this code:

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


typedef struct _thread_data_t {
        int tid;
        } thread_data_t;

void *runner(void *param);

struct thread_result
{
    int *factors;
    size_t len;
};

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

    pthread_t thr[argc];
    int i, rc;
    printf("Prime Numbers:\n");

    for(i = 1; i < argc; ++i){
        if ((rc = pthread_create(&thr[i],NULL,runner,argv[i]))){
            fprintf(stderr, "error: pthread_create, rc: %d\n", rc);
            return EXIT_FAILURE;
        }
    }


    for(i = 1; i < argc; ++i){
        void *data;
        pthread_join(thr[i], &data);

        if(data == NULL)
            continue; // error in thread

        struct thread_result *res = data;

        for(size_t r = 0; r < res->len; r++){
            printf("%d ", res->factors[r]);
        }

        free(res->factors);
        free(res);

        puts("");
    }


}

void *runner(void *param) {
    int num = atoi(param);
    if(num == 0)
        pthread_exit(NULL);

    struct thread_result *res = calloc(1, sizeof *res);

    res->factors = NULL;
    res->len = 0;

    int *tmp;

    if(res == NULL)
        pthread_exit(NULL);

    int i, j, isPrime;
    int k = 0;
    for(i=2; i<=num; i++)
    {
        if(num%i==0)
        {
            isPrime=1;
            for(j=2; j<=i/2; j++)
            {
                if(i%j==0)
                {
                        isPrime = 0;
                        break;
                }
            }

            if(isPrime==1)
            {
                tmp = realloc(res->factors, (k+1) * sizeof *res->factors);
                if(tmp == NULL)
                {
                    free(res->factors);
                    free(res);
                    pthread_exit(NULL);
                }
                res->factors = tmp;

                res->factors[k++] = i;
                res->len = k;
            }
        }
    }


    pthread_exit(res);
}

I get this output:

$ valgrind ./a 54 342 12
==17697== Memcheck, a memory error detector
==17697== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==17697== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==17697== Command: ./a 54 342 12
==17697== 
Prime Numbers:
2 3 
2 3 19 
2 3 
==17697== 
==17697== HEAP SUMMARY:
==17697==     in use at exit: 0 bytes in 0 blocks
==17697==   total heap usage: 19 allocs, 19 frees, 3,640 bytes allocated
==17697== 
==17697== All heap blocks were freed -- no leaks are possible
==17697== 
==17697== For counts of detected and suppressed errors, rerun with: -v
==17697== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

which tells me that everything worked fine and that all memory has been freed as well.

So you've must have made an error when you copied & pasted the code from my answer.