Reading Files with a double pointer and malloc

Question

So, i am making the Game of Life of Conway.I want to read a file and save it in a double pointer and malloc it's memory but i am getting a crash in codeblocks. In the world i will put the dead and the living cells.

**world = (char **)malloc(grammes * sizeof(char));
for(i=0;i<grammes;i++)
    world[i]=(char *)malloc(stiles * sizeof(char));


for(i=0;i<grammes;i++)
{
    for(j=0;j<stiles;j++)
    {
        fscanf(fp,"%c",&world[i][j]);
    }
}

for(i=0;i<grammes;i++)
{
    for(j=0;j<stiles;j++)
    {
        printf("%c",world[i][j]);
    }
    printf("\n");
}

The very first code line should probably be using `sizeof(char*)` because the third line is setting its elements to pointers. — Weather Vane, Jan 17 '18 at 18:29
@nabil.douss The var world is an universal variable.. I have declared it outside the fuction. — Achilleas Guru, Jan 17 '18 at 18:31
@WeatherVane Still codeblocks crashes when i compile&run it... It crashes when it enters at the first for. — Achilleas Guru, Jan 17 '18 at 18:32
It is impossible to store the contents of a file of any appreciable size in a pointer (including a pointer to a pointer). You may, however, be able to store the file contents in the object(s) to which the pointer points, or in other, even more indirectly-specified objects. Yes, I am in part nitpicking your wording, but it is important to understand the distinction between a pointer and the thing to which it points, and many questions around here hinge on a failure to understand it. Yours may be one of them, in fact. — John Bollinger, Jan 17 '18 at 18:49

score 2 · Accepted Answer · answered Jan 17 '18 at 18:36

2

By **world you access the actual value and not the address. You have to change it to world ( If you have already declared the variable)
In the first line it should be sizeof(char*) because world is a pointer to an array of char*

answered Jan 17 '18 at 18:36

nabil.douss

634
4
10

Achal · Answer 2 · 2018-01-17T18:47:09.950

1

Reason for crash is that world is double pointer and you should allocate memory for world first not for **world.

Replace below statement

  **world = (char **)malloc(grammes * sizeof(char));

with

 world = malloc(grammes * sizeof(char*)); //since world is double pointer it should be sizeof(char*)

Note : Typecasting of malloc is not required as suggested here Do I cast the result of malloc?

edited Jan 17 '18 at 18:47

answered Jan 17 '18 at 18:33

Achal

11,821
2
15
37

@chux I suggested the same – Achal Jan 17 '18 at 18:47
Tip: use `world = malloc(sizeof *word * grammes);` It is easier to code, review and maintain. No need to trying to get the type right. – chux - Reinstate Monica Jan 17 '18 at 19:00

user2736738 · Answer 3 · 2018-01-17T19:23:14.657

1

Well the thing is - you asked the question to misguide us not to help us to help you.

char ** world = malloc(..);

This is ok.

When you did this before char** world; and then you do this

**world = ...

Wrong. Because you have used a char to store the value of a pointer.

Well now see what you did, instead creating a chunk of memory which contains multiple char* you allocated for char then again used each of them to store the address of memory where char will be stored. Yes wrong it is

world = malloc(sizeof(char*) *grammes);

Better

world = malloc(sizeof *world * grammes);

And malloc's return value should be checked and malloc returns a void* which can be implicitly converted to char* no need to cast the result.

world = malloc(sizeof *world * grammes);
if( world == NULL ){
    perror("malloc failed");
    exit(EXIT_FAILURE);
}

Check the return value of fscanf also. You can check the man pages or standard to know their success value that hey return.

chux points out -

There are few more things but it is not sure if you will be engage in such details

malloc(sz) may return NULL if sz = 0 So saying malloc returns NULL means error is not perfectly correct or even if the sz is an overflown value then also it might return NULL.

Here it is better to write the check like this

if( world != NULL && grammes != 0){
   //Error.
}

edited Jan 17 '18 at 19:23

answered Jan 17 '18 at 19:01

user2736738

30,591
5
42
56

A pedantic note to your good answer: `if( world == NULL ){` is not necessarily a "malloc failed". Consider if `grammes == 0`, a return value of `NULL` is OK. `malloc(0)` is one of those implementation defined corner cases. I'd use `if( world == NULL && grammes != 0){` Another corner is `sizeof *world * grammes` overflowing. Yet such details are certainly beyond OP's present concern. – chux - Reinstate Monica Jan 17 '18 at 19:16
@chux.: Yes those are the nice corner points - Yes that's why safest would be to check the size first - because on size 0 malloc *may* return NULL...will add it as note. – user2736738 Jan 17 '18 at 19:18
@chux.: but again here shouldn't the check be `grammes > 0`? Ofcourse if it `size_t` then assigning `negative` will always results in true to this. – user2736738 Jan 17 '18 at 19:23
OP did not specify the sign-ness of `grammes` unfortunately. `grammes != 0` appeared safer as we lack the "sign-ness" info. With an _unsigned_ `grammes`, then either `grammes != 0` or `grammes > 0` function the same. `grammes > 0` is more clear in that case. – chux - Reinstate Monica Jan 17 '18 at 19:37

Reading Files with a double pointer and malloc

3 Answers3