CODEWITHSUNDEEP
c
GgLaPoule
GgLaPoule

Reputation: 41

C 2-dimensional array malloc do not reserve first line

I have a problem with the first line of my table

char **M = malloc(dim->lignes*sizeof(char));
for(i=0;i<dim->lignes;i++)
{
    M[i]=malloc(dim->colonnes*sizeof(char));
    for(j=0;j<dim->colonnes;j++)
    {
        M[i][j] = fgetc(F);
        printf(" ");
        printf("%c",M[i][j]);
    }
    printf("\n");
    fgetc(F);
}

for(i=0;i<dim->lignes;i++)
{
    for(j=0;j<dim->colonnes;j++)
    {
            printf(" ");
            printf("%c",M[i][j]);
    }
    printf("\n");
}

The output :

 # # # # # # # # #
 # # # # # # # # #
 #         #     #
 #   # #         #
 #               #
 #               #
 #               #
 #             # #
 #           # # #
 # # # # # # # # #

 P  Õ   h  Õ   Ç
 # # # # # # # # #
 #         #     #
 #   # #         #
 #               #
 #               #
 #               #
 #             # #
 #           # # #
 # # # # # # # # #

I don't understand why the first print is correct and not the second.

Upvotes: 2

Views: 84

Answers (4)

Roland Illig
Roland Illig

Reputation: 41676

Your code allocates not enough memory for M. Luckily, you can see the effects immediately in this special case.

The memory layout is:

M:     00 00 00 00 00 00 00 00 00
M[0]:  00 00 00 00 00 00 00 00

I'm assuming a 32 bit machine in the following. Then M is only 9 bytes large but should be 36. So when you read in line 4, the assignment to M[3] will be translated to memory[M + 4*3 .. M + 4*3 + 3] = …. The index expression is larger than the allowed 9, but the C runtime doesn't care about that. Instead it will write to the memory after the allocated M array, which happens to be M[0] your first line of characters.

Upvotes: 0

ad absurdum
ad absurdum

Reputation: 21367

You are running into problems because you allocate space for chars, which are 1 byte, when you need to allocate space for pointers to char, which are larger (8 bytes on my system):

char **M = malloc(dim->lignes*sizeof(char));

You can fix this by changing the above line to:

char **M = malloc(dim->lignes*sizeof(char *));

But, there are some common C idioms that help you avoid such mistakes. M is a pointer to a pointer to char, so you want to allocate storage for pointers to char. But *M is of type (char *), so you can write:

char **M = malloc(dim->lignes*sizeof(*M));

You can always use this pattern, which frees you from explicitly specifying the type in the right-hand side of the statement. As an added bonus, if you ever change the type of M (to (int **), for example), then the right hand side is still valid, so this idiom is more robust than the explicit statement of the type.

Further in your code, you have:

M[i]=malloc(dim->colonnes*sizeof(char));

But, in C a char is guaranteed to be 1 byte, so sizeof(char) is always 1. That makes the use of the sizeof operator redundant here. It is considered good practice to dispense with sizeof in such situations:

M[i]=malloc(dim->colonnes);

This reduces clutter in the code. You may instead consider using the first idiom, which will remain valid even if the type of M changes:

M[i]=malloc(dim->colonnes*sizeof(*M[i]));

or perhaps:

M[i]=malloc(dim->colonnes*sizeof(**M));

Upvotes: 0

chux
chux

Reputation: 154305

As well stated in this answer, the following is incorrect.

char **M = malloc(dim->lignes*sizeof(char));  // wrong with `sizeof(char)`

Rather than sizeof(char*), use sizeof *the_pointer_variable. This is less like to code wrong, It is easier to review and maintain. No need to change the code used in the malloc() argument as the type changes.

// OK
char **M = malloc(dim->lignes*sizeof(char*));
// Better
char **M = malloc(dim->lignes * sizeof *M);
// Best
char **M = malloc(sizeof *M * dim->lignes);

The last, using sizeof first, has the advantage with more complex calculations. The first one below's math is done with at least size_t math. The 2nd begins it's math with whatever type rows * columns are. Should that be int * int, overflow is possible that might not occur with size_t math.

M = malloc(sizeof *M * rows * columns);
M = malloc(rows * columns * sizeof *M);  // Overflow with rows * columns?

Upvotes: 3

t0mm13b
t0mm13b

Reputation: 34592

char **M = malloc(dim->lignes*sizeof(char));

That should read as 

char **M = malloc(dim->lignes*sizeof(char *));

It would have been valid if it was like this, this is hypothetical example,

char *M = malloc(dim->lignes*sizeof(char));

Notice the difference there, with the sizeof.

For a single pointer, use sizeof type

For a double pointer, use sizeof * type and so on.

Upvotes: 5

Related Questions