Trying to copy and allocate this pointer to work with a stack but it gives me a segmentation fault

Question

So I have a stack of

struct board_struct {
    int rows; 
    int cols;
    char board[MAX_R][MAX_C]
};

typedef struct stack_S {
    Board boards[80];
    int size;
} Stack;

typedef struct board_struct Board;
typedef struct board_struct *BoardPtr;

I have

Board
*BoardPtr
Stack stack

When I push I want the current board to be put into the stack and then the program will change it and push the new board into the stack

lets say this pop function

Board pop() {

    stack->size--;

    return stack->boards[stack->size];

}

Here is my push

void push(BoardPtr b) {

    Board n = *b;

    stack->boards[stack->size] = n;
    stack->size++;

}

The thing is that the board put into the stack has to be separated or copied from the BoardPtr and put into the Stack so that I can make changes to the BoardPtr later on. Then when I pop it I set the BoardPtr to the last board from the stack.

How can I copy the Board without changing the pointer so I can save it into a stack?

Answer 1

As others have pointed in the comments, the problem here is that you are returning the address of the local variable. This is bad, because it causes undefined behavior: in effect, you're accessing memory that no longer belongs to your program.

Here is your existing code, annotated to highlight the problem:

// Why are you passing a Board into pop? It's never used...
BoardPtr pop(Board b) {

    // result is a local variable - as soon as you leave the 
    // pop function, it disappears!
    // Also, assuming your indices are 0 based, then you are
    // returning the wrong thing: if stack->size is 1, you
    // want to return the item at position 0, not position 1!
    Board result = stack->boards[stack->size];
    stack->size--; //top is

    // Uh-oh! You are returning a pointer to result, which is
    // local. The value you return to the caller points to who-knows
    // what now!
    return &result;

}

The easiest solution is to change this function to not return a pointer to a Board, but the actual Board. Consider this:

Board pop() {
    assert (stack->size != 0);

    stack->size--;
    Board result = stack->boards[stack->size];
    return result;
} 

This works correctly because you aren't returning the local variable but a copy of it to the caller. I think that this is the best and safest option for you going forward. If you want to get fancy, you could try this version instead, which does the same thing:

Board pop() {
    assert (stack->size != 0);
    return stack->boards[--stack->size];
}

Alternatively, you could do something like this:

void pop(BoardPtr b) {
    assert (stack->size != 0);
    if (b != NULL)
        *b = stack->boards[--stack->size];
}

Caveat emptor: this last version, that accepts a BoardPtr may not work right if your struct contains pointers (see Shallow copy and deep copy in C. Yours doesn't, so this is fine.

Answer 2

If you want to preserve the base address of the board item that you want to modify its properties, you can simply copy that structure to an allocated memory region dynamically like the following:

{

 // ...

 Board realBoard = pop();                          // get Board structure at the top of your stack
 Board copyBoard = (Board*)malloc(sizeof(Board));  // allocate a Board structure in a new memory region

 memcpy(&copyBoard, &realBoard, sizeof(Board));      // simply copy it

 // do your operations on the copy version

 free(copyBoard);                                  // do not forget to free allocated memory

 // ...

}

EDIT: If you're storing copy versions of a Board structure in a Stack data structure and want to get the original one's base address, you may want to hold its memory address in every copy of it that you're storing in Stack

typedef struct tagBoard {

int rows; 
int cols;
char board[MAX_R][MAX_C];
struct tagBoard* OriginalBoardAddr;

} Board;

and your push function could be something like the following:

void push(const Board* ptr) {

stack->boards[stack->size++] = *ptr;

}

before pushing your Board (copy version) item to Stack, pass the address of the original one to store its address like the following:

{
   // Board real;   // the original Board that allocated in somewhere
   Board copy;      // the copy version that you're making differences and storing in a Stack structure

   // ...

   copy.OriginalBoardAddress = ℜ  // store original's address
   push(&copy);                        // push copy version to stack

}