Reading a file to matrix in C

Question

I get

"Debug assertion failed"

error when I try to compile this code.

Can someone please explain what is wrong with it? I think I've done something wrong with fscanf function. Thank You.

#include<stdio.h>
#include<stdlib.h>
#include<malloc.h>
void input(FILE *fp, int **a,int m)
{
    int i,j;
    for(i=0;i<m;i++)
    {
        for(j=0;j<m;j++)
        {
            fscanf(fp, "%d\n", *(a+i)+j);
        }
    }
}

int main()
{
    FILE*fp;
    int m,n,**a,i,j;
    scanf("%d",&m);
    fp=fopen("abc.txt","r");
    a=(int**)malloc(m*sizeof(int*));
    for(i=0;i<m;i++)
        *(a+i)=(int*)malloc(m*sizeof(int));
    input(fp,a,m);
    for(i=0;i<m;i++)
    {
        for(j=0;j<m;j++)
        {
            printf("%d ",*((a+i)+j));
        }
        printf("\n");
    }
    free(a);
    return 0;
}

`*((a+i)+j)` doesn't do what you think it does. You want `*(*(a + i) + j)`. — John, Jun 20 '15 at 00:02
You should be checking each call to `fscanf()` to ensure it succeeds. However, that oversight is unlikely crash your program. You don't need ``; the `malloc()` family of functions are declared in `` and you aren't using any of the extra features that `` provides. — Jonathan Leffler, Jun 20 '15 at 00:39
Not related to your current issue, but [don't cast the result of `malloc()`](http://stackoverflow.com/a/605858/3488231). — user12205, Jun 20 '15 at 00:52
regarding this line: 'printf("%d ",*((a+i)+j));' '%d' is expecting a int parameter, but the argument: '*((a+i)+j))' is a 'int*' not an 'int'. — user3629249, Jun 20 '15 at 22:31
The variable 'n' is declared in main(), but not used. Suggest, when compiling, to enable all warnings, fix the warnings. After all, the compiler is telling you there is a problem and the compiler knows the language much better than us humans. For gcc, at a minimum, use: '-Wall -Wextra -pedantic' to enable the warnings. — user3629249, Jun 20 '15 at 22:33
problems with the code: 1) always check the returned value from fscanf() (not the parameter value) to assure the operation was successful 2) in C, do not cast the returned value from malloc() (and family of functions) 3) always check (!=NULL) the returned value from malloc() to assure the operation was successful. 4) Suggest that function: input() have int return value that is set to indicate success/failure and the main() check that returned value. 5) before calling free() for array 'a', each of the entries in 'a' much be passed to free() to avoid a memory leak — user3629249, Jun 20 '15 at 22:35

donjuedo · Answer 1 · 2015-06-20T00:39:01.830

1

There are several issues here, but to directly answer your question, you are not providing an address to fscanf() where it will store the integer it finds.

Without knowing your intent, I'll give an example:

fscanf( fp, "%d\n", &(a[i]) );

That says the ith element of array a is the one to be (over)written. So if a[i] is an int pointer, you might pass &( (a[i])[j] ) in there.

Also, I think you meant to replace:

*(a+i)=(int*)malloc(m*sizeof(int));

with

a[i] = (int*) malloc(m*sizeof(int));

edited Jun 20 '15 at 00:39

answered Jun 20 '15 at 00:33

donjuedo

2,475
18
28

1

I think a[i] is already an address. Not sure if that is right. Could you explain? – Ely Jun 20 '15 at 00:37
1

But the value passed to the function is `int **a`, so `&a[i][j]` should be OK, and can be written as `*(a + i) + j` (or, if you prefer, `&(*(*(a+i)+j))` where the parentheses are not all necessary). – Jonathan Leffler Jun 20 '15 at 00:38
I think allocating as a 2 dimensional array in the first place would be more clear, and easier to handle in the code. – donjuedo Jun 20 '15 at 00:43
1

Note that `a[i]` is identical in meaning to `*(a + i)`. – Jonathan Leffler Jun 20 '15 at 00:44
True, sorry. My brain plays tricks with me when lacking sleep. – Ely Jun 20 '15 at 00:45

Ely · Answer 2 · 2015-06-20T15:18:07.773

1

The other comments and answers already pointed out what is not ok. So I skip it (fscanf(...), free, etc.).

The corrected version.

#include<stdio.h>
#include<stdlib.h>
#include<malloc.h>

void input(FILE *fp, int **a, int m) {
    int i, j;
    for (i = 0; i < m; i++) {
        for (j = 0; j < m; j++) {
            fscanf(fp, "%d\n", &a[i][j]);
        }
    }
}

int main() {
    FILE *fp;
    int m, n, **a, i, j;
    scanf("%d", &m);
    fp = fopen("abc.txt", "r");
    a = (int**) malloc(m * sizeof (int*));
    for (i = 0; i < m; i++)
        a[i] = (int*) malloc(m * sizeof (int));
    input(fp, a, m);
    for (i = 0; i < m; i++) {
        for (j = 0; j < m; j++) {
            printf("%d ", a[i][j]);
        }
        printf("\n");
    }
    for (i = 0; i < m; i++)
        free(a[i]);
    free(a);
    return 0;
}

edited Jun 20 '15 at 15:18

answered Jun 20 '15 at 00:44

Ely

10,860
4
43
64

1

Apart from a missing `fclose(fp)` (and error checking), this code runs trouble free for me. Reading the size from standard input and then the data from a fixed file name is an unusual interface, but that's how it is in the original. – Jonathan Leffler Jun 20 '15 at 03:21
You are right actually. It could be better designed. I made it short. However, if you have a structured approach for listing the "best practices" in this program please feel free to edit the post and teach the community. You seem to be very proficient in C. – Ely Jun 20 '15 at 04:17
1

There are a couple of possible approaches (at least). One is to include the dimension of the array as the first number in the file, and simply read the file. I would also make the file name an argument to the program, rather than hard-wiring it into the program. That leaves open a second alternative, which is to pass both the dimension and the file name as arguments: `matrix_reader 6 file_holding_6x6_matrix` or thereabouts. A third alternative would be to have the program read numbers until it reaches EOF, and then check that it got a square number of values. _[…continued…]_ – Jonathan Leffler Jun 20 '15 at 04:23
1

_[…continuation…]_ That means the array grows as you read, but you can grow it up to the next square number each time; allocate a 1x1 matrix, then if you read another number, allocate a 2x2 matrix, and then if you read a fifth number, allocate a 3x3 matrix, and so on. That's an unorthodox way to do it, of course. But the code shown could be told to read 25 numbers and wouldn't care that it left 11 behind in the file if it in fact held 36 numbers for a 6x6 matrix. One question is: does that matter? You can design and decide either way. – Jonathan Leffler Jun 20 '15 at 04:25

Reading a file to matrix in C

2 Answers2