Using pthread to print 2-d array

Question

So i have an assignment that says I have create a 2-d array[5][12] with random values between 1-99. Then using pthreads, I have to either add 1 or subtract 1 to each element in the array and then print the results and divide the process into either 2, 3, or 4 threads. The number of threads depends on the what the user inputs on the command line. I have code that compiles and runs. However, my desired output is only printed when the number 3 is entered. Could you guys tell me where I went wrong in my code? I'm having trouble understanding pthreads to begin with.

#include <stdio.h>
#include <stdlib.h>
#include <assert.h>
#include <ctype.h>
#include <pthread.h>
#include <iostream>

using namespace std;
int list[5][12];
int rows = 5;
int cols = 12;
int threadc;

void *threadf(void *arg)
{
    int x = (int) arg;
    for(int i = (x*60)/threadc; i < ((x+1) * 60)/threadc; i++)
    {
        for(int j = 0; j < 12; j++)
        {
            if (list[i][j] % 2 == 0)
                list[i][j] += 1;
            else
                list[i][j] -= 1;
        }
    }
}

void cArray()
{
    srand(time(NULL));
    for(int i = 0; i < 5; i++)
    {
        for(int j = 0; j < 12; j++)
        {
            list[i][j] = rand() % 99 + 1;
        }
    }

}

void pArray(int list[][12], int rows, int cols)
{
    cout << "\n";
    for(int i = 0; i < rows; i++)
    {
        for(int j = 0; j < cols; j++)
        {
            cout << list[i][j] << " ";
        }
        cout << "\n";
    }
 }

int main(int argc, char *argv[])
{
    if(argc != 2) exit(0);
    threadc = atoi(argv[1]);
    assert(threadc >= 2 && threadc <=4);
    pthread_t *thread;
    thread = new pthread_t [threadc];
    if(thread == NULL)
        exit(0);
    cArray();
    cout << "2-d Array: ";
    pArray(list, rows, cols);
    int t;
    for(int i = 0; i < threadc; i++)
    {
        t = pthread_create(&thread[i], NULL, threadf, (void *)i);
        if (t != 0)
            return 1;
    }
    for(int i = 0; i < threadc; i++)
    {
        t = pthread_join(thread[i], NULL);
        if(t != 0)
            return 1;
    }  
    cout << "Modified 2-d Array: ";
    pArray(list, rows, cols);
    return 0;
}

Answer 1

Lets take a look at the outer for loop in threadf for x = 0 and threadc = 4

    for(int i = (0*60)/4; i < ((0+1) * 60)/4; i++)
    for(int i = 0; i < (1 * 60)/4; i++)
    for(int i = 0; i < 60/4; i++)
    for(int i = 0; i < 15; i++)

i ranges from 0 to 14. i is being used like this: list[i][j], so consider where writes to list[14][11] will go. Well outside the bounds defined by int list[5][12]; Bad smurf will happen. Undefined behaviour, so technically no one knows what will happen. We can make some pretty good guesses though.

int list[5][12];
int rows = 5; // probably overwritten by write to list[6][0]
int cols = 12; // probably overwritten by write to list[6][1]
int threadc; // probably overwritten by write to list[6][3]

So row and column are moving, but no one cares. The code never uses them. But threadc... That's used all over the place. In fact, it's used in the loop exit condition. More badness is likely to occur here. It also determines the number of threads to be created and joined. Some threads may not be created. The program may try to join more threads than exist.

Anyway, Undefined Behaviour. I suppose we should all be glad that the compiler didn't generate code that ordered a tactical nuke strike. Since this is a homework question I'm not going to unravel the math OP requires to make their for loop correctly divvy up the work across multiple threads, but will suggest that they look into treating the array as a 1D array of size 5*12 and doing the 1D->2D indexing themselves in a single for loop.

Other notes:

use uintptr_t instead of int for i in main and x in threadf. uintptr_t is guaranteed to convert to a void *

Use an unsigned variable like size_t for the loop counters and array indexers. They play nicely with uintptr_t and you almost never want a negative array index.

Use a std::vector instead of a pointer and new for the thread list. If you have to use new and a pointer, remember to delete the list when you're done with it.

See if you can use std::thread instead of the pthreads.

Add a return to threadf.

Answer 2

Somehow thread_join returns an error code(22 in my case) when the number of threads created is 2. If you remove the return statement in the second loop, your program prints the final output.

for(int i = 0; i < threadc; i++)
{
    t = pthread_join(thread[i], NULL);
    if (t != 0) 
        return 1; // <- your program works if you comment out this.


}

According to this link: http://minirighi.sourceforge.net/html/errno_8h.html 22 is EINVAL which means 'thread is un-joinable'.

Since you care about the return value of pthread_join, I would advise to add a successful termination function (pthread_exit(NULL);) at the end of your thredf. Also, to avoid buffer overrun as mentioned by @user4581301, you could pass a pointer to the data.

So thredf would be like

void *threadf(void *arg)
{
    cout << endl;
    int x = *((int*)arg); // <- NOTICE HERE!
    for(int i = (x*60)/threadc; i < ((x+1) * 60)/threadc; i++)
      //...
    }
    pthread_exit(NULL); // <- NOTICE HERE!
}

And the main:

int main(int argc, char *argv[])
{
    if(argc != 2) exit(0);
    threadc = atoi(argv[1]);
    assert(threadc >= 2 && threadc <=4);
    pthread_t *thread;
    thread = new pthread_t [threadc];
    int *data = new int[threadc]; // <- NOTICE HERE!

    if(thread == NULL)
        exit(0);
    cArray();
    cout << "2-d Array: ";
    pArray(list, rows, cols);
    int t;
    for(int i = 0; i < threadc; i++)
    {
        data[i] = i;
        //                                                NOTICE HERE!
        t = pthread_create(&thread[i], NULL, threadf, (void *)(&data[i])); 
        if (t != 0)
            return 1;
    }
    // ...