How do I use malloc for a two dimensional array of structs? (bus error: 10)

Question

I'm trying to implement a sudoku solver. To do this i am using a struct, shown below, to represent a cell on a sudoku board. I am then declaring a 9x9 array of these structs to represent the board.

cell structure:

struct cell{
     char value;
     unsigned int possible;
};

then, i declare an array of sctructs as:

struct cell board[9][9];

My problem is, when i try to enter a value into the array, (ie. board[2][2].value = getchar();), sometimes it works, and other times i get this error:

Bus error: 10

I'm not quite sure what this means... How is "Bus error: 10" different from a segmentation fault?

I'm using gcc and just editing in vim. My feelings are, I need to dynamically allocate the memory for this array. Now, I understand how to use malloc to allocate memory for a two dimensional array, something like this:

int ** Array;  
Array = (int**) malloc(x_size*sizeof(int*));  
for (int i = 0; i < x_size; i++)  
    Array[i] = (int*) malloc(y_size*sizeof(int)); 

But I'm having trouble implementing the memory allocation part for a 2 dimensional array of structs.

Would it something like this?

struct cell** board;
board = (struct cell**) malloc(x_size*sizeof(struct cell**));
for(int i=0; i< x_size; i++)
    board[i] = (struct cell*) malloc(y_size*sizeof(struct cell));

I'm wary that this " sizeof(struct cell) " is not properly allocating the amount of memory it should be.

Any help would be greatly appreciated! I'm fairly new to C (C++ is my native tongue), I've used embedded C a good amount, but I am trying to better grasp the language as a whole.
Bonus points for detailed\in depth explanations!

Thanks!

EDIT OK, so thanks everyone for the great suggestions, I still haven't implemented any dynamic memory allocation, but, as requested, here is the code that is producing the bus error:

 /* only code being used in solver.h*/
 29 /* structure to describe a cell */
 30 struct cell{
 31     int value;
 32     unsigned int possible;
 33 };



   /*solver.c*/
 4 #include <ctype.h>
 5 #include <stdio.h>
 6 #include "solver.h"
 7 
 8 
 9 struct cell board [9][9];
 10 
 11 
 12 int main(){
 13     initialize_board();
 14     print_board();
 15     setup_board();
 16     print_board();
 17 return 0;
 18 }
 19 
 20 void print_board(){
 21     int i=0, j=0;
 22     for(i = 0; i<9; i++){
 23         for(j = 0; j<9; j++)
 24             printf(" %d",board[i][j].value);
 25         printf("\n");
 26     }
 27 }
 28 
 29 void initialize_board(){
 30     int i = 0, j = 0;
 31 
 32     for(i = 0; i<9; i++)
 33         for(j = 0; j<9; j++){
 34             (board[i][j]).value = 0;
 35             (board[i][j]).possible = 0x1FF;
 36         }
 37 }
 38 
 39 void setup_board(){
 40     int row=0, col=0, val = 0;
 41     char another = 'Y';
 42 
 43     printf("Board Initial Setup.\nEnter the row and column number of the value to be entered into the board.");
 44     printf("\nRow and Column indexes start at one, from top left corner.");
 45     while(another == 'Y'){
 46         printf("\nRow: ");
 47         row = getchar();
 48         printf("Column: ");
 49         getchar();
 50         col = getchar();
 51         printf("Value: ");
 52         getchar();
 53         (board[row-1][[col-1]).value = getchar();
 54         printf("Enter another value? (y/n): ");
 55         getchar();
 56         another = toupper(getchar());
 57         getchar();
 58     }
 59 }

As you can see, i've changed the datatype of value to int to match the getchar() return type. But my code is still producing strange runtime errors/results. For example, in the first iteration of the while loop in setup_board, I can enter say Row:1, Col:1, Value:5, then when i enter 'n' to exit, the board should be printed with 5 in the upper left corner, however this is not the case. The matrix that is printed is still in its state after initialize_board() is called.

Output:

Board Initial Setup.
Enter the row and column number of the value to be entered into the board.
Row and Column indexes start at one, from top left corner.
Row: 1
Column: 1
Value: 4
Enter another value? (y/n): n
 0 0 0 0 0 0 0 0 0
 0 0 0 0 0 0 0 0 0
 0 0 0 0 0 0 0 0 0
 0 0 0 0 0 0 0 0 0
 0 0 0 0 0 0 0 0 0
 0 0 0 0 0 0 0 0 0
 0 0 0 0 0 0 0 0 0
 0 0 0 0 0 0 0 0 0
 0 0 0 0 0 0 0 0 0

Also, if i enter other matrix coordinates, thats when i get the Bus Error: output:

Board Initial Setup.
Enter the row and column number of the value to be entered into the board.
Row and Column indexes start at one, from top left corner.
Row: 5
Column: 5
Value: 5
Bus error: 10

Any advice on how to clean up that ugly double getchar() business would be appreciated too.
Thanks everyone!

EDIT NUMBER TWO The problem was with those getchar()'s.... I didn't realize they return an integer ASCII code to represent the numbers instead of the actual numbers themselves. Here is what I've done to fix it:

 47     while(another == 'Y'){
 48         valid=0;
 49         while(!valid){
 50             printf("\nRow: ");
 51             row = getchar() - '0';  /* convert ASCII character code to actual integer value */
 52             getchar();              /*added to remove extra newline character from stdin */
 53             printf("Column: ");
 54             col = getchar() - '0';
 55             getchar();              /*remove \n */
 56             printf("Value: ");
 57             val = getchar() - '0';
 58             getchar();              /*remove \n */
 59             if(val >9 || val<1 || col>9 ||col<1 || row>9 || row<1)
 61                 printf("\nInvalid input, all values must be between 1 and 9, inclusive");
 62             else
 63                 valid = 1;  
 64         }
 65         board[row-1][col-1].value = val;
 66         printf("Enter another value? (y/n): ");
 67         another = toupper(getchar());
 68         getchar();                  /*remove \n */
 69     }

Thanks to everyone for your help and input, I will be implementing many of the suggestions you've all made and have learned a lot just from this question!

EDIT NUMBER THREE

ONE FINAL QUESTION!

Though my original issue is solved, does anyone have strong opinions or reasoning as to whether or not it would be better to implement the matrix by dynamically allocating the memory?

I figure, I'll leave it as is now since it works, but since the matrix is fairly large, would it be better programming practice to dynamically allocate?

Answer 1

getchar() returns an int. Perhaps the value returned exceeds the range of char. See getchar()

Answer 2

First, some notes on idioms. There is an idiom to malloc arrays in C:

Type *ptr;
ptr = malloc (n * sizeof(*ptr));

The sizeof operator can receive not only the type, but also a variable with that type. Note the asterisk before ptr, which means we are allocating something with the size of Type, not Type*. There is no need to cast the return, as a void* pointer can be assigned to any pointer. That way, you can define a macro to alloc any array as follows:

#define ALLOC(p, n) p = malloc(n * sizeof(*p))

Also, when allocating bi- or multi- dimensional matrices, it is usual to fetch all memory you need at once, as follows:

Type **matrix;
matrix = malloc(row * sizeof(*matrix));
matrix[0] = malloc(row * col * sizeof(*matrix[0]))
for (i=0; i < row; i++){
    matrix[i] = matrix[0] + i*col;
}

That way we do only two allocations, one to fetch the row header pointers, and the other to fetch all the needed memory. After, we make all row header pointers point to the correct place in the matrix, so we can use usual idioms as matrix[i][j]. Some also like to allocate a single vector and access with matrix[i*col + j], but I find the first much more clear.

Lastly, it is not a settled matter but I find way easier to define the struct as a type, and then not need to remind all the time that it is indeed a struct

typedef struct Cell Cell;
struct Cell{
    ...
};

Cell board[9][9];

Lastly, I tested your static Cell board and didn't find that weird bus error. That may be due to char padding, but I find it unlikely. Can it be something due to the getchar? It will pick newlines and spaces.

Answer 3

getchar returns a code that represents a character; you have to add code that converts it to a number. Consider, for example, the following code:

printf("\nRow: ");
row = getchar();
printf("Column: ");
getchar();

What happens when the user types "hehe" instead of 1 and 1 that the program expects? The program will get ASCII codes for h and e, assign them to row and col and so access out-of-range array elements.

Actually, the same happens with normal input! The user enters 1, the program gets the ASCII code (49) and performs some memory overrun:

board[49-1][49-1].value = 53;

To fix this, convert character codes to numbers:

if (row >= '1' && row <= '9')
    row -= '0'; // a common C hack for converting a character-code to a number
else
    printf("Bad input"); // not a proper error handling, just an example

col -= '0'; // 1 line for brevity; you must do the same code as above

value = getchar();
value -= '0'; // same as above

board[row-1][col-1].value = value;

Answer 4

Beside the Q, as I see it has been answered, but regarding your comments on malloc etc.

malloc return pointer to uninitialised newly-allocated space for an object of size size.

You do not receive a space of for example int, but a space of size N. This is why you can take a char and read the bytes in an int.

Now to some points being subjective and perhaps a bit off SO, but give it a go.

---

Casting malloc is redundant and only clutter up the code. It is like instead of filling a bottle with water you fill a bottle with water to fill a bottle of water. The allocated memory does not become what it is casted to. It is a chunk of memory. Period. In some way it is looking at it backwards.

An int * points to a specific part of virtual memory and treat that memory based on chunks by size of int. Depending on byte order that would be LSB first or last in a 4 byte sequence on a specific system having 4 byte int's.

---

When it comes to macros I would not recommend using macros for functions like malloc. It makes the code a pain to read and file-operations like grep etc. becomes useless/harder.

When one comes across a macro one has to check what that macro does. One add one more layer of error prone code etc. When you after 6 months or if someone other then you read the code you'll have to check what that macro actually does. If you read malloc you know what that does, when you read MYMALLOC you'll have no idea.

It is basic building blocks best kept as is. No gain considered the pain.

Sometimes one come across code like:

BLAH(k, v);
while (--i)
     BOFF(mak, VED(uu));
if (FOO != k)
      BAR;

Now, to decipher you'll have to read the macros which often is the definition of obscurity and it quickly becomes a mess. You know the primitives and standard functions. Don't hide what is happening.

And never ever change the control flow of a program using macros.

---

When it comes to typedefs I generally hate them. I never typedef struct's.

Say i.e. struct foo bar; vs BLAH bar;. Now in the latter bar can be anything. A function, an unsigned char, a struct, a pointer to a primitive etc. As the code grows this becomes another thing to keep track on. The former, using struct is concise and short and anyone who reads it know it is a struct, and it is not a pointer.

In large it often does more to obfuscate code then make it more clear. Don't hide what is happening.

Typedefs can be useful for i.e. function pointers, but then again function pointers should also be used with care.

---

Same goes for #define. Use it scarcely.

Answer 5

I don't know why you get the Bus 10: error, it should work fine with the static array.

But for the dynamic allocation of memory for the board you could use:

cell ** board = (cell **) malloc(x_size * sizeof(cell *));

for (int i = 0; i < Size_X; i++)
{
    board[i] = (cell *) malloc (y_size * sizeof (cell));
}

You can also allocated the board as a simple array of 81 cell:

cell * board = (cell *) malloc (x_size * y_size (cell));

I've tested the static allocation under Windows and it works fine, as I did test the above with malloc, but my compiler is set to C++ so above code might not be 100% working with a pure C compiler.

Hope it helps.