0

I am trying to write a simple C++ algorithm to solve sudoku. I am trying to pass address values between different functions but i get segmentation fault at runtime. (Needless to say, i am not quite experienced :))

The code does manage to pass the address of a[0] to main function and i can read values using pointers inside main. When i try to pass the address to solve function, it gives segmentation fault.

(Also as a secondary question, i can read values correctly in main, using cout << *(a+5) etc. correctly (commented out in main), but when i try to print all 81 values stored using a for loop, it gives out nonsense values (again, commented out in code). The code works with literals like *(a+3) or a[3], but does not when an int gets involved for(int i, whatever) cout << *(a+i);)

#include <iostream>
using namespace std;

int * get_input();
void solve(int *);

int main()
{
    int * a;
    a = get_input();
    //cout << *a << " " << *(a+1) << " " << *(a+2) << " " << *(a+3) << " " << *(a+4);
    //for (int i = 0 ; i < 81 ; i++) {if (i%9 == 0) cout << "\n"; cout << a[i] << " ";}
    solve(a);
    return(0);
}

int * get_input ()
{
    int a[81];
    getinput:
    for (int i = 0 ; i < 81 ; i++)  {a[i] = 0;}
    for (int i = 0 ; i < 81 ; i++)  {cin >> a[i];}
    print:
    for (int i = 0 ; i < 81 ; i++)
    {
        if (i%27 == 0){cout << "\n";}
        if (i%9 == 0) {cout << "\n";}
        if (i%3 == 0) {cout << "  " << a[i];}
        if (i%3 != 0) {cout << a[i];}
    }
    cout << "\n\nCheck:\n1- Fix\n2- Reset\n3- Confirm\n\n";
    int check = 0;
    cin >> check;
    if (check == 1)
    {   
        int input[3] = {-1, -1, -1};
        while (true)
        {
            cin >> input[0] >> input[1] >> input [2];
            if (input[1] == 0) goto print;
            a[(input[2]-1)+((input[1]-1)*9)] = input[0];
        }
    }
    if (check == 2) goto getinput;
    if (check == 3) return a;
}

void solve(int * a)
{
    bool matrix[9][9][9];
    for (int i = 0 ; i < 81 ; i++) {for (int j = 0 ; j < 9 ; j++) {matrix[(i-i%9)/9][i%9][j] = true;}}
    for (int i = 0 ; i < 81 ; i++)
    {
        if (a[i] == 0) continue;
        else
        {
            for (int j = 0 ; j < 9 ; i++)
            {
                matrix[(i-i%9)/9][j][a[i]] = false;
                matrix[j][i%9][a[i]] = false;
                matrix[((i-i%9)/9)-((i-i%9)/9)%3+j%3][i%9-(i%9)%3+(j-j%3)/3][a[i]] = false;
            }
        }
    }
    for (int i = 0 ; i < 9 ; i++)
    {
        for (int j = 0 ; j < 9 ; j++)
        {
            cout << matrix[i][j][1] << " ";
        }
        cout << "\n";
    }
}
corsel
  • 315
  • 2
  • 12

4 Answers4

3

You are returning an address to a local variable in getInput(the array a). I would suggest you pass the array as argument to this function. Another option is to allocate the array dynamically and then take care to free it before the program terminates.

Ivaylo Strandjev
  • 69,226
  • 18
  • 123
  • 176
2

Make the a array in your get_input() function static:

int a[81];

should be 

static int a[81];

This works because the static keyword ensures that the allocated memory block (the array a ) will remain allocated after the function returns. Usually this is "because I'm not finished with it yet" (for example, you can count how many times your function is called this way), but it can also, legitimately, be used to ensure that the return value of a function survives the end of the function.

Marginally better would be to declare the array at the main level, and pass a pointer to it into the get_input() and solve() functions. That way you make it explicit in the code that the array will "live for the duration of the program" - and that's usually good practice.

Floris
  • 45,857
  • 6
  • 70
  • 122
  • I can't deny it... but since the "puzzle so far" should be stored in memory all the time, a static isn't bad. It would be better to put it at the top level than in a function. – Floris Feb 18 '13 at 13:35
  • Of course. I see why it would work and I see your rationale for it, I just can't shake the feeling that it feels a little nasty :P. It might be worth adding *why* this works though. – slugonamission Feb 18 '13 at 13:37
  • 1
    Accessing a static through a function is perfectly legit, even desirable since in some cases (not this one) it neatly avoids the static initialization fiasco. The only issues with using a static like this are to do with threading and reentrancy and I don't see that being a problem in this case. – Benj Feb 18 '13 at 13:37
  • Thanks, i will try this. This was my first question in stackoverflow and honestly, i'm overwhelmed with how many legit responses are given in such short time :) – corsel Feb 18 '13 at 16:42
  • @cem - you're welcome. As a new user you can never quite know if you're going to be down voted and closed in under a minute, or get 5 useable responses in five minutes. You lucked out! By the way - please please please learn how to write structured loops that don't need "goto" - it is rarely the right approach (although it can be). See http://stackoverflow.com/questions/245742/examples-of-good-gotos-in-c-or-c – Floris Feb 18 '13 at 16:49
  • alright, i will take a look at that. goto has impressed me with its simple, old-school look but i guess it's not good practice for larger projects. – corsel Feb 18 '13 at 18:21
  • oh i wasn't aware there is a "team goto" and rivals in the geek world :) thread made me fear the world war 3 might start with a fight over goto. – corsel Feb 18 '13 at 19:00
  • At least now you are warned... Happy hacking! – Floris Feb 18 '13 at 19:13
  • Thank you! stayed up till 5 am to figure this out. – killer Mar 17 '18 at 10:03
1
int a[81];

This is a local memory allocation, and its freed when your function get_input returns.

Use a pointer int* a, and malloc function to allocate dynamically memory instead!

The malloc command may is like this(if i remember it well):

int *a = (int *) malloc(sizeof(int)*81);
Paschalis
  • 11,929
  • 9
  • 52
  • 82
0

You're problem is that you are returning a pointer to a locally declared variable. Don't do this. You should either pass in the variable as a parameter (eg. get_input(int[] arr, int length)` or allocate new memory on the heap for your array. The simplest is the former, the latter can get you into trouble as you have to manage your memory or you will get memory leaks.

Why do you need to do this? When you declare a[] in get_input it allocates the space for that variable on the stack. The stack is a long contiguous block of memory that is used to store a function's parameters, it's local variables and the address of the program that is calling the current function. When a function returns, all this memory is reclaimed for use by the next function call. That is, when solve is called it starts writing over the memory on the stack that was previously used by get_input.

You are quite lucky that you got a segmentation fault as it is sometimes possible for programs to continue operating even though their data has become utterly corrupted.

In summary: Declare your array in the main function and pass it to get_input to operate on.

Dunes
  • 37,291
  • 7
  • 81
  • 97