printing strings produces garbage even when (I think) they are null terminated

Question

When I run print_puzzle(create_puzzle(input)), I get a bunch of gobbledegook at the bottom of the output, only in the last row. I have no idea why this keeps happening. The output is supposed to be 9 rows of 9 numbers (the input is a sudoku puzzle with zeroes representing empty spaces).

This bunch of code should take that input, make a 2d array of strings and then, with print_puzzle, print those strings out in a grid. They are string because eventually I will implement a way to display all the values the square could possibly be. But for now, when I print it out, things are screwed up. I even tried putting the null value in every single element of all 81 strings but it still get's screwed up when it goes to print the strings. I'm lost!

typedef struct square {
  char vals[10]; // string of possible values
} square_t;

typedef struct puzzle {
  square_t squares[9][9];
} puzzle_t;

static puzzle_t *create_puzzle(unsigned char vals[9][9]) {
  puzzle_t puz;
  puzzle_t *p = &puz;
  int i, j, k, valnum;
  for (i = 0; i < 9; i++) {
    for (j = 0; j < 9; j++) {
      puz.squares[i][j].vals[0] = '\0';
      puz.squares[i][j].vals[1] = '\0';
      puz.squares[i][j].vals[2] = '\0';
      puz.squares[i][j].vals[3] = '\0';
      puz.squares[i][j].vals[4] = '\0';
      puz.squares[i][j].vals[5] = '\0';
      puz.squares[i][j].vals[6] = '\0';
      puz.squares[i][j].vals[7] = '\0';
      puz.squares[i][j].vals[8] = '\0';
      puz.squares[i][j].vals[9] = '\0';
      valnum = vals[i][j] -'0';
      for (k = 0; k < 10; k++){
        if ((char)(k + '0') == (char)(valnum + '0')){
          char tmpStr[2] = {(char)(valnum +'0'),'\0'};
          strcat(puz.squares[i][j].vals, tmpStr);
        }
      }
    }
  }
  return p;
}

void print_puzzle(puzzle_t *p) {
  int i, j;
  for (i=0; i<9; i++) {
    for (j=0; j<9; j++) {
      printf(" %2s", p->squares[i][j].vals);
    }
    printf("\n");
  }
}

Answer 1

In function create_puzzle(), you are returning a pointer of the type puzzle_t. But, the address of variable puz of the type puzzle_t is invalid once you return from the function.

Variables that are declared inside a function are local variables. They can be used only by statements that are inside that function. These Local variables are not known to functions outside their own, so returning an address of a local variable doesn't make sense as when the function returns, the local storage it was using on the stack is considered invalid by the program, though it may not get cleared right away. Logically, the value at puz is indeterminate, and accessing it results in undefined behavior.

You can make puz a global variable, and use it the way you are doing right now.

Answer 2

In short: In function create_puzzle(), you are returning a pointer to the local variable puz. Local variables are only known to function inside their own. So the content referenced by the pointer returned by create_puzzle is indeterminate.

More details: In C++, local variables are usually generated as storage on a "stack" data structure. when create_puzzle() method is entered, its local variables come alive. A function's local variables will be dead when the method is over. An implementation of C++ is not required to leave the garbage you left on the stack untouched so that you can access it's original content. C++ is not a safe language, implementations let you make mistake and get away with it. Other memory-safe languages solve this problem by restricting your power. For example in C# you can take the address of a local, but the language is cleverly designed so that it is impossible to use it after the lifetime of the local ends.

This answer is very awesome: Can a local variable's memory be accessed outside its scope?

Answer 3

You are returning a local variable here:

return p;

Declare p and puz outside of the function, then it should work.

Answer 4

p point to local memory that is unavailable after the function ends. Returning that leads to problems. Instead allocate memory.

// puzzle_t puz;
// puzzle_t *p = &puz;
puzzle_t *p = malloc(sizeof *p);
assert(p);

Be sure to free() the memory after the calling code completes using it.