Where are leaks?

Hello, i need help from more intelligent people than i am :) I have in my school project leaks, but i dont know where. When i run some piece of code i found leak here in allocation.

Code:

---

stStruct** myArray = NULL;


void allocMyArray()
{
    if (myArray == NULL) {
        myStruct = (stStruct**)malloc(sizeof(stStruct*)*dimense);
        for (int i = 0; i < dimense; i++) {
            myArray[i] = (stStruct*)malloc(sizeof(stStruct));
        }
    }
}


void deleteMyArray()
{
    if (myArray != NULL) {
        for (int i = 0; i < dimense; i++) {
            free(myArray[i]);
        }
        free(myArray);
        myArray = NULL;
    }
}

---

Maybe is leak on another place .. :/
I will be very happy for any help

You should avoid global variables and clean up on failure. So, I might expect something like this:

Code:

---

stStruct **allocMyArray(size_t dimense)
{
    stStruct **myArray = malloc(sizeof(myArray[0]) * dimense);
    if (!myArray) {
        return NULL;
    }

    for (size_t i = 0; i < dimense; i++) {
        myArray[i] = malloc(sizeof(*myArray[i]));
        if (!myArray[i]) {
            for (size_t j = 0; j < i; j++) {
                free(myArray[j]);
            }
            return NULL;
        }
    }
    return myArray;
}

---

Having said that, do you really need a dynamic array of stStruct pointers, each of which points to a lone stStruct object rather than to the first object in a dynamic array of stStruct objects? This can be useful if you want to allow for "gaps" in the array such that an object can be removed from the array by setting the corresponding pointer to be a null pointer. But if not, it would be more efficient to simply create a dynamic array of stStruct objects rather than pointers.

You might also consider creating a struct to bundle the dynamic array with its dimension, e.g.,

Code:

---

struct stStructArray
{
    stStruct **data;
    size_t dimense;
};

---

This can help avoid mistakes where you have more than one such dynamic array, and then you mix up which dimension is for which dynamic array.

Quote:

---

Originally Posted by laserlight

You should avoid global variables and clean up on failure. So, I might expect something like this:

Code:

---

stStruct **allocMyArray(size_t dimense)
{
    stStruct **myArray = malloc(sizeof(myArray[0]) * dimense);
    if (!myArray) {
        return NULL;
    }

    for (size_t i = 0; i < dimense; i++) {
        myArray[i] = malloc(sizeof(*myArray[i]));
        if (!myArray[i]) {
            for (size_t j = 0; j < i; j++) {
                free(myArray[j]);
            }
            return NULL;
        }
    }
    return myArray;
}

---

Having said that, do you really need a dynamic array of stStruct pointers, each of which points to a lone stStruct object rather than to the first object in a dynamic array of stStruct objects? This can be useful if you want to allow for "gaps" in the array such that an object can be removed from the array by setting the corresponding pointer to be a null pointer. But if not, it would be more efficient to simply create a dynamic array of stStruct objects rather than pointers.

You might also consider creating a struct to bundle the dynamic array with its dimension, e.g.,

Code:

---

struct stStructArray
{
    stStruct **data;
    size_t dimense;
};

---

This can help avoid mistakes where you have more than one such dynamic array, and then you mix up which dimension is for which dynamic array.

---

Just to prove the power of code reviews :D

Code:

---

stStruct **allocMyArray(size_t dimense)
{
    stStruct **myArray = malloc(sizeof(myArray[0]) * dimense);
    if (!myArray) {
        return NULL;
    }

    for (size_t i = 0; i < dimense; i++) {
        myArray[i] = malloc(sizeof(*myArray[i]));
        if (!myArray[i]) {
            for (size_t j = 0; j < i; j++) {
                free(myArray[j]);
            }
            free(myArray);  // <<< ADDED THIS
            return NULL;
        }
    }
    return myArray;
}

---

Thanks a lot, i repaired my code. But still have leaks :/ Last option where i think can be leak is that, i have struct which store 2 another strucs. How i should free this struc correct? I can add code, but there is lot, so if u can add some your example i will be glad.

"example"

Code:

---

typedef struct a{
 int id;
 char name[50];
 char tel[12];
}stA

typedef struct b{
 int id;
 char str[50];
}stB

typedef struct c{
 int id;
 stA* myA;
 stB* myB;
}stC

---

That depends on how you allocate them.

Have you changed your global variables into local variables? It will be easier to reason about your code that way.

We have in our assignment, so i cant change it

Code:

---

static stStruct** myArray = NULL;

---

For two first struct i read data from file and load them to the separate linked list. I have in these two struct pointer on next node, that i build this linked list and i have static variable on actual node and first node. There is no problem. When i want get these two struct like a parameters to third struct. First i allocated memory only for third struct(how i showed in my allocated example). Then i call function, where i allocated memory for struct A and B and created new object, because i need stored these data after i dealocated linked list for these structs.

I apologize for the way I am trying to explain, my english is not so good, but i trying do my best. I can add my code on github if u want, maybe it will be easier.
Thank u good people

edit:
Maybe could you show me on my previous 3 struct, how would you do this array.
I have 3 structs. A, B, C .. A and B i load from file. Then i allocated array for objects C for example size is 2. When i created i call function find A and B by ID and then i add into object C and this object add to array ..

Here's what I would do if I were you:

• For each component that you need dynamic memory allocation, create a function to allocate (and possibly initialise), and another function to free the allocated memory. You already have an example of this with allocMyArray and deleteMyArray.
• If a component builds on smaller components that require dynamic memory allocation, the allocator function will not call malloc directly, but rather it will call the allocator function of the smaller components. Likewise, the deallocator function will not call free directly, but rather it will call the deallocator function of the smaller components.
• Test that these functions work independently of the rest of your program, e.g., for each component write a tiny test program that just creates and destroys a component object. This is also where you can use your leak checker tool to check that the simplest use of such a pair of functions do indeed result in no memory leaks.
• Combine the use of these components and their allocator/deallocator functions into your actual program.