1

I'm working on a project (NOT HOMEWORK), building a multi-thread sudoku solution validator in C. I'm new to C so excuse the bad code quality as I'm still improving.

I want to call the method row_check 9 times from 9 separate threads. For the method as parameters I pass the row number (arg) and array name (arr). I have created the thread but I'm unsure how to pass the parameters properly to the method. Can anyone help me with this?

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


void* row_check(void* arg, int *arr)
{
    int i = *((int *)arg); //trying to convert row number to int
    int j, flag;

    while(i < 9)
    {
        flag=0x0000;

        for(j = 0; j < 9; j++)
            flag |= 1 << (arr[i][j]-1);

        if (flag != 0x01FF)
            report("row", i, j-1);
    }

}

void report(char *s, int i, int j)
{
   printf("\nThe sudoku is INCORRECT");
   printf("\nin %s. Row:%d,Column:%d", s, i+1, j+1);
   getch();
   exit(0);
 }


int main(int argc, char* argv[])
{
     int i,j;
     char arr1[9][9];
     FILE *file = fopen(argv[1], "r");

     if (file == 0)
     {
       fprintf(stderr, "failed");
       exit(1);
     }
      int col=0, row=0;
      int num;

      while(fscanf(file, "%d ", &num) == 1)
      {
         arr1[row][col] = num;
         col++;
         if(col == 9)
         {
            row++;
            col = 0;
         }
      }
      fclose(file);

      pthread_t tid;
      pthread_attr_t attr; 
      pthread_attr_init(&attr);

      int n;
      for(n=0; n < 9; n++) //creating 9 threads
      {
          pthread_create(&tid, &attr, row_check, n);
          pthread_join(tid, NULL);
      }

      return 0;
}
Alex Lop.
  • 6,810
  • 1
  • 26
  • 45
Joel Shevin
  • 103
  • 1
  • 2
  • 9
  • You can use structure to store two variables and pas that structure to the thread routine. Do you want to pass the array name only i.e. a string or you want to do something with the values of the array? – Gaurav Pathak Apr 15 '17 at 20:24
  • 1
    Read the manual for `pthread_join`, because when you call it like that it is the same as if you called `row_check` in your `for` loop. `pthread_join` waits until thread is finished, so these threads don't run at the same time but one after another. Also, the third argument passed to `report` in `row_check` (the `j-1`) is always 8 because after the `for` loop is finished `j` is equal to 9 – Rogus Apr 15 '17 at 20:33
  • I am not getting a good feeling about the while loop in the code!! Can you explain about it? – Gaurav Pathak Apr 15 '17 at 21:10
  • @GauravPathak i need to access the values in the 2d array row wise. and check if the row has all the values from 1 to 9. thats what the while loop is used for. – Joel Shevin Apr 16 '17 at 06:15
  • @Rogus yes i need each thread to finish its work and report the validity. so one thread has to wait for the other to finish it so its done in order – Joel Shevin Apr 16 '17 at 06:16
  • then why are you using threads? can't you just call `row_check` instead of creating a thread and waiting for it? – Rogus Apr 16 '17 at 09:27
  • @Rogus this needs to be multithread . – Joel Shevin Apr 16 '17 at 12:28
  • Even though you're using threads you are not taking advantage of the power multithreading gives. When your main thread creates a thread running your `row_check` function the main thread stops and only the created thread runs. There are (almost) never two threads running at the same time in your application. That's not how multithreading works. It would be the same (and even better since no additional resources are acquired) if you just called `row_check` in your loop. I don't understand why are you trying to force it (and it's not homework), it's like buying a gamimg computer to play solitaire – Rogus Apr 16 '17 at 19:43

1 Answers1

0

The thread entry function has to be of the format void *(*start_routine) (void *), which means it receives only one parameter - pointer to anything you like.

The most used technique is to define a struct with the values you want to pass to the thread entry function. Create a variable of that type, initialize it and pass its address to the thread entry function.

Example:

typedef thread_data_s
{
    char *ptr;
    int   row_num; // I would prefer to define it as `unsigned int` but I stick to your example
    // + any other data you want to pass to the thread
} thread_data_t;

....

thread_data_t data[NUM_OF_THREADS];

....

for(n=0; n < NUM_OF_THREADS; n++) //creating 9 threads
{
     data[n].ptr = &arr1[n][0];
     data[n].row_num = n;
     pthread_create(&tid, &attr, row_check, &data[n]);
}

...

for(n=0; n < NUM_OF_THREADS; n++) // waiting for all the threads here
{
     pthread_join(tid, NULL);
}

And your entry function should look something like this:

void* row_check(void* data)
{
    //int i = *((int *)arg); //trying to convert row number to int
    thread_data_t *my_data_ptr = data;
    int j, flag;

    while(i < 9)
    {
        flag=0x0000;

        for(j = 0; j < 9; j++)
            flag |= 1u << ( (my_data_ptr->ptr)[my_data_ptr->row_num][j] - 1 );
        // Shouldn't it be under the `for` loop block? If so, please add `{}` to the `for` loop
        if (flag != 0x01FF)
            report("row", my_data_ptr->row_num, j-1);
    }

    return NULL;
}
Alex Lop.
  • 6,810
  • 1
  • 26
  • 45
  • `thread_data_t *my_data_ptr = data;` typecasting is required. – Gaurav Pathak Apr 15 '17 at 20:53
  • @GauravPathak I am not sure typecasting `void *` to another pointer type is required in *C*. But I'll check it later and fix if needed. – Alex Lop. Apr 15 '17 at 21:07
  • Let me know through your comment. please. I am also curious to know about it. Thx. – Gaurav Pathak Apr 15 '17 at 21:17
  • @GauravPathak I did a quick search, and according to the answer to the following question, type casting is not required http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc – Alex Lop. Apr 15 '17 at 21:30
  • @AlexLop. hi i modified the code as you have mentioned , i know i must be just a general idea of how it should be done but since im very new things seems a bit complex. after i compiled i got the follow errors [link](https://ibb.co/khhHO5) . could you please help me with this ? – Joel Shevin Apr 16 '17 at 12:27
  • @JoelShevin where did you define `thread_data_t`? Looks like it doesn't recognize this type – Alex Lop. Apr 16 '17 at 12:45
  • @AlexLop. u can view my modified code [here](https://drive.google.com/open?id=0B5-ZflH_obS4b1pWMjFlbXNCbFU) – Joel Shevin Apr 16 '17 at 12:48
  • @JoelShevin put the definition of thread_data_t right after the includes – Alex Lop. Apr 16 '17 at 12:53
  • @AlexLop. u mean the typedef thread_data_s code ? , if so i just put it right after includes , still getting 17 errors . :/ – Joel Shevin Apr 16 '17 at 12:59
  • @JoelShevin put a link – Alex Lop. Apr 16 '17 at 13:00
  • this is the [code](https://drive.google.com/open?id=0B5-ZflH_obS4Um5uVWU2QWJHTTQ) [error list](https://ibb.co/f5jE35) – Joel Shevin Apr 16 '17 at 13:07
  • @JoelShevin Joel, come on, do me a favor! Did you look what is written there? It points to the basic syntax errors like missing the specifier struct, missing `;`...try to fix the basic errors and wornings by yourself. Let me know if there is really something not clear. – Alex Lop. Apr 16 '17 at 13:15
  • @AlexLop. managed to fix most of the errors , only a few remains and im not sure how to fix them. would really appreciate some help. :) [errors](https://ibb.co/m8vU35) [code](https://drive.google.com/open?id=0B5-ZflH_obS4LTdkNWUwRXM5QUk) – Joel Shevin Apr 16 '17 at 14:19
  • @JoelShevin 1. `while(my_data_ptr < 9)` you are comparing pointer with 9. Probably should be `my_data_ptr->raw_num`... but you never decrease it so it will be an endless loop in the current implementation. 2. Define `ptr` in the `thread_data_t` as `char**`, not as `char*`. 3. Change `void report(char *s,int w,int q)` to `void report(const char *s,int w,int q)` and add this line `void report(const char *s,int w,int q);` right after the `thread_data_t` struct definition. Add `#include` to your include list. – Alex Lop. Apr 16 '17 at 15:51
  • @JoelShevin OK, I think I see what you were trying to do. Don't change the `char*` to `char**` in `thread_data_t`. Instead, replace the body of `check_raw` with the following code: thread_data_t *my_data_ptr = data; int j, flag; flag=0x0000; for(j = 0; j < 9; j++) { flag |= 1u << ( (my_data_ptr->ptr)[j] - 1 ); if (flag != 0x01FF) { report("row", my_data_ptr->row_num, j-1); } } return NULL; Just add "new line" where needed. – Alex Lop. Apr 16 '17 at 16:04
  • @AlexLop. awesome thanks , now all the errors are gone and code seems to be alright but in my input file i changed a number to a character , since the row check function check that only numbers from 1 to 9 are there it should show an error. but it shows nothing and exits , any suggestions ? – Joel Shevin Apr 16 '17 at 16:44
  • @JoelShevin send me the link to your updated code. I will take a look. – Alex Lop. Apr 16 '17 at 17:30
  • @AlexLop.[here](https://drive.google.com/open?id=0B5-ZflH_obS4LTdkNWUwRXM5QUk) , btw i cant use conio for some reason so im using ncurses. – Joel Shevin Apr 16 '17 at 17:37
  • @AlexLop. check this segment on my code if its correct. int NUM_OF_THREADS=9; thread_data_t data[NUM_OF_THREADS]; pthread_t tid; pthread_attr_t attr – Joel Shevin Apr 16 '17 at 17:44
  • @AlexLop. btw in the input file the values should be give with spaces . eg: 1 2 4 5 7 ... – Joel Shevin Apr 16 '17 at 17:58
  • @JoelShevin NUM_OF_THREADS usually defined as macro or constant global. Move its definition after the includes like this `const unsigned int NUM_OF_THREADS = 9;` Regarding the problem with the exit: you read from the file only numbers (`%d`), once `fscanf(..., "%d",...)` sees a character, it returns 0 and the `while` loop breaks. You need to read it as characters with `"%c"` instead of `"%d"`, which means that if it reads "1", the actual value of it will be the ASCI code of "1". – Alex Lop. Apr 16 '17 at 18:58
  • @JoelShevin These questions are good for asking on StackOverflow. You better post it as a separate question and you will get response much faster since I am not always available. BTW, it the issue with the threads is resolved, you need to mark my answer as the "accepted answer" by clicking on the green V beside it. – Alex Lop. Apr 16 '17 at 19:01
  • @AlexLop. thanks a lot i marked it as the answer, will open a new question. see if you can join it as well. thanks again. – Joel Shevin Apr 17 '17 at 00:12