I will left an example below so you can compare it to the way you wrote it originally...
About your code:
- Declare loop variables inside the
for
command
- May be
Smaller
do not need to be a pointer
- Keep dimensions as variables. It is more flexible
- You did not set the values for
rows
and cols
in the struct
. And in main()
do not use fixed values as 3 and 4 as you did
- You should set all cells to different values, not the same. You will feel safer when you see reversible values, like 100*
row
+ column
in the example... This way you can see if the loops are ok and all elements are being printed. See this output for printArray()
:
0 1 2 3
100 101 102 103
200 201 202 203
Each line starts with the line number so you can test it a few times before going on.
- make your program test itself. In
printArray()
for example show the dimensions like this:
printArray[3,4]
0 1 2 3
100 101 102 103
200 201 202 203
See the output of the example
- always write the code to free the memory, in the reserve order of the allocation, maybe in a separate function that returns NULL in order to invalidate the pointer back in the calling code, like this
Smaller* freeArray(Smaller* A)
{
printf("\nfreeArray()\n");
for (int i = 0; i < A->rows; i++)
{
free(A->array[i]); // delete lines
printf("row %d free()\n", i);
}
free(A->array); // delete cols
printf("pointer to rows free()\n");
free(A); // delete struct
printf("struct free()\n");
return NULL;
}
This way you know that the pointer sand
will not be left pointing to an area that has been free()
d. Using such a pointer will crash your program so it may be good to write
sand = freeArray(sand);
output of the example code
printArray[3,4]
0 1 2 3
100 101 102 103
200 201 202 203
freeArray()
row 0 free()
row 1 free()
row 2 free()
pointer to rows free()
struct free()
Example code
#include <stdio.h>
#include <stdlib.h>
typedef struct
{
int** array;
int rows, cols;
} Smaller;
void fillArray(Smaller*);
Smaller* freeArray(Smaller*);
Smaller* makeArray(size_t, size_t);
void printArray(Smaller*);
int main(void)
{
int y = 3;
int x = 4;
// sand points to a Smaller
Smaller* sand = makeArray(y, x);
// adding known unique values to cells is easier
fillArray(sand);
printArray(sand); // show values
sand = freeArray(sand); // delete all
return 0;
}
void fillArray(Smaller* A)
{
for (int i = 0; i < A->rows; i++)
for (int j = 0; j < A->cols; j++)
A->array[i][j] = 100 * i + j;
}
Smaller* freeArray(Smaller* A)
{
printf("\nfreeArray()\n");
for (int i = 0; i < A->rows; i++)
{
free(A->array[i]); // delete lines
printf("row %d free()\n", i);
}
free(A->array); // delete cols
printf("pointer to rows free()\n");
free(A); // delete struct
printf("struct free()\n");
return NULL;
}
Smaller* makeArray(size_t y, size_t x)
{
// sand points to a Smaller
Smaller* sand = (Smaller*)malloc(sizeof(Smaller));
sand->rows = y;
sand->cols = x;
// allocate mem for number of rows, that is 'y'
sand->array = malloc(y * sizeof(int*));
// allocate mem for each of the 'x' columns
for (size_t i = 0; i < y; i++)
sand->array[i] = malloc(x * sizeof(int));
return sand;
};
void printArray(Smaller* sand)
{
printf("printArray[%d,%d]\n\n", sand->rows, sand->cols);
for (int i = 0; i < sand->rows; i++)
{
for (int j = 0; j < sand->cols; j++)
printf("%3d ", sand->array[i][j]);
printf("\n");
}
}
About the code
Please SO people do not bother pointing me not to cast the result of malloc()
. It is by decision. This common recommendation is a reminiscence of the C-faq of the 90's and now we know that implicit conversions maybe not so good. In fact implicit things may cost you a lot of time: if you malloc()
a series of different structs in a program and omit the types if some of them are for example reversed keep in mind that the use of all casts would help you avoid this costly type of mistake...