Deallocation of memory in arrays

Question

I've done a program that opens the file (read binary), and saves all the words(in the file) in an array of char (allocated dynamically in base of the length of the word).
This is the code:

char **leggi_stringhe(const char *filename, size_t *size) {
    FILE *f = fopen(filename, "rb");
    if (f == NULL) {
        *size = 0;
        return NULL;
    }

    int x;

    if (fread(&x, 1, 4, f) != 4) {
        *size = 0;
        return NULL;
    }

    char **stringhe = malloc((x) * sizeof(char));

    for (int i = 0; i < x; i++) {
        int z = 0;
        if (fread(&z, 1, 4, f) != 4) {
            *size = 0;
            return NULL;
        }

        stringhe[i] = malloc((z)* sizeof(char));
        if (fread(stringhe[i], 1, z, f) != z) {
            *size = 0;
            return NULL;
        }
        stringhe[i][z] = 0;
    }
    *size = x;
    fclose(f);
    return stringhe;
}

int main(void) {
    size_t t;
    char **a = leggi_stringhe("file1.bin", &t);

    for (int i = 0; i < t; i++)
        free(a[i]);
    free(a);;
}

The program works, but i have problems with memory deallocation. After calling of leggi_stringhe function, the variable a contains:

a[0] = "first"
a[1] = "second"
a[2] = "third"

but when i'm trying to deallocate the whole a variable as i wrote, the debugger stops with a warning.
I was inspired by this question for writing my code Using Dynamic Memory allocation for arrays, but do not understand why I get this error when i try to deallocate.

fread(&z, 1, 4, f)? Why do you assume that z is four bytes? You also implicitly assume that your data file comes from a machine with the same endianness as the one you're processing on. — Andrew Henle
– Andrew Henle, Commented Feb 15, 2017 at 14:07
This code has other fundamental problems apart from the mentioned bugs. See Correctly allocating multi-dimensional arrays. — Lundin
– Lundin, Commented Feb 15, 2017 at 14:14

Some programmer dude · Accepted Answer · 2017-02-15 14:06:33Z

4

Your initial call to malloc is wrong. You allocate space for x characters, not for pointers to char.

Your second call inside the loop is wrong to, as you don't allocate space for the terminator.

Lastly, and unrelated to the problems you ask about, but if the fread calls inside the loop fails, you will have memory leaks.

answered Feb 15, 2017 at 14:06

Some programmer dude

411k36 gold badges420 silver badges655 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Iharob Al Asimi Over a year ago

To the OP: @M41Npain and please check that malloc() did not return NULL. It might be too much for this code, but it's a habit that you MUST develop

Amarildo Over a year ago

@IharobAlAsimi cplusplus.com say If the function failed to allocate the requested block of memory, a null pointer is returned. what it is referring?

Some programmer dude Over a year ago

@M41Npain In C NULL is the standard symbol for a null pointer. If, in your case, stringhe is equal to NULL after the first call to malloc, then the allocation failed and you should not continue using the pointer.

RoadRunner · Accepted Answer · 2017-02-15 14:20:49Z

1

There are some problems with your code:

This line:

char **stringhe = malloc((x) * sizeof(char));

Needs to be:

char **stringhe = malloc((x) * sizeof(char*)); /* or sizeof *stringhe */

As you need to allocate x char* pointers for stringhe.

Inside your first for loop, you are not adding +1 for null-terminator. It needs to be instead:
```
stringhe[i] = malloc(z+1); /* sizeof(char) = 1 */
```
You need to check return of malloc(). It can return NULL if unsuccessful. You can do this by simply checking if (ptr == NULL), then exit the program. It would unsafe to allow a failed malloc() to continue in the program.
for (int i = 0; i < t; i++) is comparing an int with size_t. This should be for (size_t i = 0; i < t; i++) instead.

answered Feb 15, 2017 at 14:20

RoadRunner

26.4k6 gold badges46 silver badges77 bronze badges

3 Comments

Amarildo Over a year ago

I did not add +1 in stringhe[i] = malloc((z)* sizeof(char)); because after the malloc i see with the the debugger, that the malloc function always allocates more space that is necessary, in fact then I added stringhe[i][z]= 0; to cut the rest

RoadRunner Over a year ago

@M41Npain When allocating char* pointers, it is always safe to add that extra +1 when using malloc(). It may work now, but in the future, if you don't do this, and malloc() does not allocate enough space for the \0, then you will be accessing beyond the bounds of what was allocated.

RoadRunner Over a year ago

@M41Npain When dealing with heap allocation, its better to be safe then sorry. I'm glad I could help :).

Collectives™ on Stack Overflow

Deallocation of memory in arrays

2 Answers 2

3 Comments

3 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

3 Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related