-1

I'm trying to make a program in C that generates a random sudoku. It generates random number and checks if in the row or in the col or in the square 3x3 there is the same number, if not it puts it in the cell e goes to the next. The only problem is with row 5, when the index is 6 it gives Segmentation Fault. If I change as I comment in the program, it goes in loop. What is wrong?

include <string.h>
#include <stdlib.h>
#include "sudoku.h"
#include <stdio.h>
#include <time.h>

int dimension = 9;

int main(int argc, char** argv){
  int dimension = 9;
  int j ,k ;
  int ** sudo =  malloc(sizeof(*sudo)*dimension);
  for ( j = 0; j< dimension; j++){
    sudo[j] = malloc(sizeof(int)*dimension);
    for ( k = 0; k<dimension; k++){
      sudo[j][k] =0;

    }
  }
  riempiSudoku(sudo);
  return 0;


}

void riempiSudoku(int** sudo){  //fill sudoku
  int i,j;
  srand ( time(NULL));
  srand(rand());
  for (i=0;i<dimension;i++){
    for(j=0;j<dimension;j++){
      int ran;
      do
    ran= rand() %9 ;
      while(checkSquare(sudo,i,j,ran+1)||checkRow(sudo,i,ran+1)
        ||checkCol(sudo,j,ran+1));
      sudo[i][j] = ran+1;
      printf("%d", sudo[i][j]);
    }
    printf("\n");
  }
}


int checkRow(int** sudo, int row, int value){ //check if the number is in the row
  int i;
  for (i = 0; i<dimension; i++){
    if (sudo[row][i] == value)
      return 1;
  }
  return 0;
}

int checkCol(int** sudo, int col, int value){//check if the number is in the col
  int i;
  for (i = 0; i<dimension; i++){
    if (sudo[i][col] == value)
      return 1;
  }
  return 0;

}

int checkSquare(int** sudo, int row, int col, int value){ //check if the number is in the square 3x3
  int i,j;
  if (row==0||row==2||row==1){
    if(col==0||col==1||col==2){
      for(i=0;i<3;i++){
    for(j=0;j<3;j++){
      if (sudo[i][j] == value)
        return 1;
    }
      }
      return 0;
    }
    if(col==3||col==4||col==5){
      for(i=0;i<3;i++){
    for(j=3;j<6;j++){
      if (sudo[i][j] == value)
        return 1;
    }
      }
      return 0;
    }
    if(col==6||col==7||col==8){
      for(i=0;i<3;i++){
    for(j=6;j<9;j++){
      if (sudo[i][j] == value)
        return 1;
    }
      }
      return 0;
    }
  }
  if (row==3||row==4||row==5){
    if(col==0||col==1||col==2){
      for(i=3;i<6;i++){
    for(j=0;j<3;j++){
      if (sudo[i][j] == value)
        return 1;
    }
      }
      return 0;
    }
    if(col==3||col==4||col==5){
      for(i=3;i<6;i++){
    for(j=3;j<6;j++){
      if (sudo[i][j] == value)
        return 1;
    }
      }
      return 0;
    }
    if(col==6||col==7||col==8){
      for(i=3;i<6;i++){
    for(j=6;j<9;j++){
      if (sudo[i][j] == value)
        return 1;
    }
      }
      return 0;
    }
  }
  if (row==6||row==7||row==8){
    if(col==0||col==1||col==2){
      for(i=6;i<9;i++){
    for(j=0;j<3;j++){
      if (sudo[i][j] == value)
        return 1;
    }
      }
      return 0;
    }
    if(col==3||col==4||col==5){
      for(i=6;i<9;i++){
    for(j=3;j<6;j++){
      if (sudo[i][j] == value)
        return 1;
    }
      }
      return 0;
    }
    if(col==6||col==7||col==8){
      for(i=6;i<9;i++){
    for(j=6;j<9;j++){
      if (sudo[i][j] == value)
        return 1;
    }
      }
      return 0;
    }
  }


}
Leonardo
  • 499
  • 1
  • 7
  • 18
  • 2
    Why don't you allocate the right amount of memory, for a change? `int ** sudo = (int**) malloc(sizeof(int)*dimension);` is wrong, not just because you shouldn't cast the result of `malloc()` (and you *shouldn't*), but because you need `sizeof(int*)`. – EOF Aug 22 '15 at 23:09
  • I tried to change with int** sudo = (int**) malloc(sizeof(int*)*dimension) and now it prints some row, but at some points it goes in loop and I have to stop with ctrl+c – Leonardo Aug 22 '15 at 23:24
  • 1
    Well, did you ensure that your algorithm will always find a solution and terminate? I strongly suspect your approach is horribly flawed. – EOF Aug 22 '15 at 23:26
  • `sizeof(int*)` is not _necessarily_ the same as `sizeof(int)`. – ryyker Aug 22 '15 at 23:44
  • In theory it shuold work, once it printed 8 of 9 rows ,but it never terminate correctly. The rows that it prints are correct, all different number in row, col or square – Leonardo Aug 22 '15 at 23:50
  • Hint: you don't need 2d arrays. Hint2: don't cast malloc. hint3: static allocation works fine for 9*9 arrays. – wildplasser Aug 22 '15 at 23:51

1 Answers1

1

In C, it is not correct to cast the return of [m][c][re]alloc()
C99 (or any other C version that I am aware of) does not require a cast. C implicitly casts to and from void *. The cast then is done automagically.
C++ on the other hand requires a cast as it will only convert to void *, not from

For starters then change this section of your code: 

 int ** sudo = (int**) malloc(sizeof(int)*dimension);
  for ( j = 0; j< dimension; j++){
    sudo[j] = (int*) malloc(sizeof(int)*dimension);

To: 

 int ** sudo = malloc(sizeof(*sudo)*dimension);
  for ( j = 0; j< dimension; j++){
    sudo[j] = malloc(sizeof(int)*dimension);

Note: for the first malloc, memory size needed for pointer space is very dependent on the target executable, i.e. 32 or 64 bit, but sizeof(*sudo) in this case, will work for either.

Community
  • 1
  • 1
ryyker
  • 22,849
  • 3
  • 43
  • 87
  • I tried as you said but still getting in loop, sometimes it prints 1 row, sometimes it prints 3 or 5, but it never terminates – Leonardo Aug 23 '15 at 00:11
  • @Leonardo - what part of code exactly are you seeing the wrong looping? You've got lots of loops. – ryyker Aug 23 '15 at 00:14
  • I don't know, I tried with gdb doing step by step but I can't get nothing. In theory all the loops should stop when it generates a number that fill correctly the row,col and square – Leonardo Aug 23 '15 at 00:25
  • @Leonardo - Specifically, it appears the `do-while` loop in the `riempiSudoku()` function never leaves. in pseudo code form you have `do {get new random number;} while(A==true||B==true||C==true);` So, for any loop, as long as A or B or C is true, you will stay in the loop. Is this what you intend? – ryyker Aug 24 '15 at 00:59