I want to initialise and print a 2-D array in two separate functions, but i don't really know if i am doing this correctly.
I wrote 2 functions: int** matrix_initialization(int n, int m)
and int print_matrix(int n1, int n2, int **a);
In the first function i have to arguments: int n - the number of rows and int m - the number of cols. In this function i initialise a pointer to the pointer int **matrix = NULL;
Then i allocate memory to it, and giving this 2-D array random values.
Is that okay that the type of the int** matrix_initialization(int n, int m)
function is int **
?
The second function is int print_matrix(int n1, int n2, int** a)
and there are some problems, the main is that it is not working. I have three arguments int n1
- rows, int n2
- cols, int** a
a pointer to the pointer. This function is to print matrix.
Here is my code:
#include <stdio.h>
#include <locale.h>
#include <stdlib.h>
#include <time.h>
int** matrix_initialization(int n, int m)
{
int** matrix = NULL;
matrix = (int**)malloc(n * sizeof(n));
if (matrix != NULL)
{
if (matrix != NULL)
{
for (int i = 0; i < m; i )
{
*(matrix i) = (int*)malloc(m * sizeof(m));
}
for (int i = 0; i < n; i )
{
for (int j = 0; j < m; j )
{
matrix[i][j] = rand() % 20 - 5;
}
}
}
}
return matrix;
}
int print_matrix(int n1, int n2, int** a)
{
for (int i = 0; i < n1; i )
{
for (int j = 0; j < n2; j )
{
printf("%d\t", a[i][j]);
}
printf("\n");
}
}
int main()
{
srand(time(NULL));
int N, M, **a;
scanf_s("%d %d", &N, &M);
a = matrix_initialization(N, M);
print_matrix(N, M, a);
}
CodePudding user response:
You have two major problems:
The first is
malloc(n * sizeof(n))
which is the same as
malloc(n * sizeof(int))
And that makes no sense when you want to create an array of n
pointers to int
. On a 64-bit system it's unlikely that sizeof(int) == sizeof(int *)
.
It should be
malloc(n * sizeof(int*))
Or better yet
malloc(n * sizeof *matrix)
The second problem is your loop where you allocate the int
arrays:
for (int i = 0; i < m; i )
You have just created an array of n
elements. Now you iterate over m
elements? This will only work if n == m
.
The correct loop should of course be
// Iterating over an array of n elements
for (int i = 0; i < n; i )
You also have the same problem with the malloc
call as mentioned above, but this time it just so happens to work because m
is an int
which is the right type and size.
On another few notes, in C you should not cast the result of malloc
.
You never free
any of the data you allocate, leading to memory leaks.
And instead of *(matrix i)
use matrix[i]
.
CodePudding user response:
There are many issues with this code.
Major:
- You aren't using 2D arrays but arrays of pointers. That doesn't make the slightest sense for this application, you are just making your code more complex, error prone and slow for absolutely nothing gained. See Correctly allocating multi-dimensional arrays.
matrix = (int**)malloc(n * sizeof(n));
Both of your malloc calls are strange,n
is just a size parameter (could as well besize_t
) and not necessarily of the same type as the data. Which is aint*
in this case, not anint
. You should be allocating something likematrix = malloc(n * sizeof(*matrix))
- The first for loop
for (int i = 0; i < m; i )
is wrong, should ben
. - You aren't freeing allocated memory.
Minor:
- locale.h isn't needed for this program.
- There's no actual need to initialize your pointer
matrix
to NULL if you are to assign to it on the next line. Also you have multiple redundant checks vs NULL. *(matrix i)
is never the best way to access an array in C, it's just an obscure way of writingmatrix[i]
. Don't complicate things just for the heck of it.- Don't name parameters
n
andm
in one function and thenn1
n2
in another function. Don't complicate things just for the heck of it. - Avoid functions ending with
_s
unless you have specific reasons why you need to use them. They are non-portable. It's very likely that whoever told you to use it has no idea what they are talking about. Including no idea about the whole "bounds-checking interface" debacle in C11. int main()
is obsolete style, always useint main (void)
(as opposed to C ).
Here is a rewrite with all problems fixed, using 2D arrays instead:
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
void* matrix_initialization (int n, int m)
{
int (*matrix)[m] = malloc( sizeof(int[n][m]) );
if (matrix != NULL)
{
for (int i = 0; i < n; i )
{
for (int j = 0; j < m; j )
{
matrix[i][j] = rand() % 20 - 5;
}
}
}
return matrix;
}
void print_matrix (int n, int m, int a[n][m])
{
for (int i = 0; i < n; i )
{
for (int j = 0; j < n; j )
{
printf("%d\t", a[i][j]);
}
printf("\n");
}
}
int main (void)
{
srand(time(NULL));
int N, M;
scanf("%d %d", &N, &M);
int (*a)[M];
a = matrix_initialization(N, M);
print_matrix(N, M, a);
free(a);
}