dynamic allocation of 2d array function
Asked Answered
T

2

-4

So I have a program in C structured in 3 files: main.c, alloc.h and alloc.c. In the main.c function, I have the declaration of a pointer to another pointer to which I intend to alloc an n * m array:

#include <stdio.h>
#include <stdlib.h>
#include "alloc.h"

int main() {
   int **mat, n, m;
   alloc_matrix(&mat, int &n, int &m);
   return 0;
}

In alloc.c I have the following declarations:

#ifndef ALLOC_H_INCLUDED
#define ALLOC_H_INCLUDED
#include <stdio.h>
#include <stdlib.h>
void alloc_matrix(int***, int*, int*);
#endif

In alloc.c I have the function:

void alloc_matrix(int ***mat, int *n, int *m) {
    printf("\nn = "); scanf("%d", n);
    printf("\nm = "); scanf("%d", m);
    *mat = (int**)calloc(*n, sizeof(int*));
    int i;
    for (i = 0; i < *n; i++)
        *(mat + i) = (int*)calloc(*m, sizeof(int));
}

But the program doesn't work. It enters some kind of loop and doesn't end. If I allocate it in main it would work but I have no idea what I am doing wrong in the alloc function.

Trigger answered 28/11, 2016 at 21:49 Comment(9)
There is no 2D array, nor a pointer to one or something which can represent one! And being a 3-star C programmer is not a compliment.Anaemia
If that was your code: please do not correct its mistakes in the question. If it was not, please repost the whole question with the actual code.Amieva
alloc_matrix(&mat,int &n,int &m); this is not legal C. What compiler are you using?Hattie
i'm using codeblocksTrigger
it runs for me,but the problem is that i can't read the matrixTrigger
What works for you? The code you posted, or the code you changed after a (now deleted) comment?Amieva
Code::Blocks is an IDE, not a compiler. We are still waiting for the necessary information. We are no debugging service.Anaemia
The code of main as posted cannot possibly be accepted by a C compiler. The code of alloc_matrix as posted should provoke a warning, which is in your situation as good as an error. Work on understanding and fixing it.Tweak your IDE settings so that warnings are treated as errors. You want to add -Wall -Wextra -Werror to the compiler flags. It's the only way to develop in C and stay sane.Hattie
You'll make life easier for yourself if you return int **, instead of using an int *** parameterSingles
N
2

Here is the correct code. Your error was that in the definition of alloc_matrix, you used *(mat+i) in the allocation loop, which should be *(*mat+i) as, mat is a int*** so the base address for the 2D array would be in *mat. Then you need to move by offset i and then de-reference that memory location for the 1D array.

Main:

#include <stdio.h>
#include <stdlib.h>
#include "alloc.h"
int main()
{
   int **mat,n,m;
   alloc_matrix(&mat,&n,&m);
   return 0;
}

alloc.h

#ifndef ALLOC_H_INCLUDED
#define ALLOC_H_INCLUDED
#include <stdio.h>
#include <stdlib.h>
void alloc_matrix(int***,int*,int*);

#endif

alloc.c :

void alloc_matrix(int ***mat,int *n,int *m)
{
    printf("\nn = "); scanf("%d", n);
    printf("\nm = "); scanf("%d", m);
    *mat = (int**)calloc(*n,sizeof(int*));
    int i;
    for(i = 0; i < *n; i++)
    *(*mat+i) = (int*)calloc(*m,sizeof(int));
}

The code for the read function :

void read_matrix(int ***mat,int n,int m)
    {
      int i,j;
      for(i = 0; i < n; i++)
       for(j = 0; j < m; j++)
        {
          printf("mat[%d][%d] = ", i, j);
          scanf("%d", (*(*mat+i))+j);
        }
    }

The problem with it is that it only reads the first row and the it freezes.

Nook answered 28/11, 2016 at 22:6 Comment(2)
It seems to work.But when i try to read it it only reads the first row.What can be the problem with the way i read it ?void read_matrix(int ***mat,int n,int m) { int i,j; for(i = 0; i < n; i++) for(j = 0; j < m; j++) { printf("mat[%d][%d] = ", i, j); scanf("%d", *(*(mat+i)+j)); } }Trigger
@Trigger void read_matrix(int *** The problem is the three stars. You don't need that many. The other problem is ` (*(mat+i)+j));. This is just nonsense. A matrix is accessed like this: **mat[i][j]`*.Hattie
H
2
void alloc_matrix(int ***mat,int *n,int *m)

There are two problems in this line. Neither is fatal but both are worth fixing.

First problem: A matrix in this program is represented as an int**. Why does alloc_matrix accept an int***? All standard functions that allocate something (malloc and friends) return a pointer to that something. This is an idiomatic way of doing things in C. It reduces your star count (being a three-star C programmer is not an achievement to be proud of) and simplifies the code. The function should be changed to

int** alloc_matrix( // but what's inside the () ?

The second problem is, why should a function called alloc_matrix prompt the user and read values? These things are not related to allocation. A function should do one thing and do it well. Does malloc prompts you to enter the size? Does fopen prompt you to enter the filename? These things would be regarded as nonsense of the first degree, and rightly so. It is advised to read the sizes elsewhere and pass them to alloc_matrix as input arguments. Hence,

int** alloc_matrix(int n, int m) { // but what's inside the {}?

What remains of alloc_matrix is simple:

int** alloc_matrix(int n, int m) {
  int** mat; // that's what we will return
  int i;
  mat = (int**)calloc(n, sizeof(int*));
  for(i = 0; i < n; i++)
     // here comes the important part.

Since we have simplified alloc_matrixand reduced the star count in mat, what should we do with the old body of the loop? It was:

    *(mat+i) = (int*)calloc(...);

but if we remove a star, it becomes

    (mat+i) = (int*)calloc(...);

which is an obvious nonsense. Perhaps the old line was a problem. The fact that it provoked a compiler warning certainly doesn't speak for its correctness. So how to correct it? There aren't too many options. It turns out that in order to restore sanity, we must leave the old left-hand side (written for the three-star mat) intact. Or, better still, use an equivalent but more idiomatic notation:

    mat[i] = (int*)calloc(m, sizeof(int));

So the entire function now becomes

int** alloc_matrix(int n, int m) {
  int **mat;
  int i;
  mat = (int**)calloc(n, sizeof(int*));
  for(i = 0; i < n; i++)
    mat[i] = (int*)calloc(m, sizeof(int));
  return mat; 
}

and it should be called like

mat = alloc_matrix(n, m);

It is often said that one should not cast the result of calloc and friends. But in this case the cast has enabled a warning which helped to find a bug. I'm leaving the casts in place for now.

There is another idiom for the allocation that does not require the cast, but also avoids the problem of types not matching. Instead of using the type for the sizeof, you can use the dereferenced pointer as the type information is available in the variable:

mat = (int**)calloc(n, sizeof(int*));

can be changed to

mat = calloc(n, sizeof *mat); //sizeof is an operator not a function
Hattie answered 28/11, 2016 at 22:51 Comment(7)
Could you please explain precisely how the cast enabled this warning, and what the warning is?Vinson
@BradenBest The cast produced a warning because the type on the left side of the assignment didn't match the type on the right side. It turned out that the left hand side was wrong.Hattie
ah, to catch newbie errors. Although I'm pretty sure that if one fumbles the level of indirection on the type, they're probably going to fumble the cast as well. Aside from that, I think it would be in the askers's best interest to reduce the level of indirection and use a better design. An int ** is hardly useful for a matrix, IMO.Vinson
Why not use mat = calloc(n, sizeof mat);? It is much easier to read and avoids the problem from the start.Adlar
@KamiKaze I guess that's because mat = calloc(n, sizeof mat); would be incorrect.Hattie
Sry I meant mat = calloc(n, sizeof *mat); And if that does not make sense either now I blame the heat....Adlar
@KamiKaze this is correct, some people prefer it this way. I'm just not used to this styls. Please feel free to edit the answer.Hattie

© 2022 - 2024 — McMap. All rights reserved.