Nested structures allocation / deallocation causing crash

Question

I'm back into C after a long break, and I said to myself "what about learning some deep learning ?". So, I have these structures:

• Neuron, containing and array of inputs, an array of weights and the number of inputs,
• NeuralLayer, containing an array of neurons, and the number of neurons,
• NeuralNetwork, containing a List of NeuralLayer.

Previously the network was an array of layers, but since the number of layers might be unknown, I now use a list.

I wanted the network be used as follow: create, add layer by layer, output, delete.

// A neuron is an array of inputs, an array of weights, and the size of its arrays
typedef struct Neuron {
    double * inputs;    
    double * weights;
    size_t inputsCount;
} Neuron;




// Creates a Neuron and returns it
Neuron * Neuron_create (size_t inputsCount) { 
    Neuron * neuron;
    size_t weightsIndex;

    neuron = Structure_create (sizeof (Neuron));

    neuron->inputsCount = inputsCount;
    neuron->inputs = (double *) Structure_create (neuron->inputsCount * sizeof (double));
    neuron->weights = (double *) Structure_create (neuron->inputsCount * sizeof (double));

    for (weightsIndex = 0; weightsIndex < inputsCount; weightsIndex++) 
        neuron->weights [weightsIndex] = rand () / (RAND_MAX / 1.0); // On creation, random weights are used

    return neuron;
}


// Deletes the neuron and sets its pointer to NULL
void Neuron_delete (Neuron ** target) {
    if ((NULL == target) || (NULL == * target))
        return;

    printf ("    Deleting neuron: %p\n", * target);

    Structure_delete (& (* target)->inputs); 
    Structure_delete (& (* target)->weights); 
    Structure_delete (target);

    printf ("    Neuron deleted\n");
}

---

// A layer is an array of Neurons and its size
typedef struct NeuralLayer {

    size_t neuronsCount;
    Neuron ** neurons;
} NeuralLayer;




// Creates a Layer and returns it
NeuralLayer * NeuralLayer_create (size_t neuronsCount, size_t inputsCount) {

    NeuralLayer * layer;
    size_t neuronsIndex;

    layer = Structure_create (sizeof (NeuralLayer));
    layer->neuronsCount = neuronsCount;
    layer->neurons = Structure_create (layer->neuronsCount * sizeof (NeuralLayer *));

    for (neuronsIndex = 0; neuronsIndex < neuronsCount; neuronsIndex++)
        layer->neurons [neuronsIndex] = Neuron_create (inputsCount); 

    return layer;
}


// Deletes the layer and sets its pointer to NULL
void NeuralLayer_delete (NeuralLayer ** target) {

    if ((NULL == target) || (NULL == * target))
        return;

    printf ("  Deleting layer: %p\n", * target);

    for (size_t neuronIndex = 0; neuronIndex < (* target)->neuronsCount; neuronIndex++) {
        Neuron * buffer = (* target)->neurons [neuronIndex];
        Neuron_delete (& buffer);
    }      

    Structure_delete ((* target)->neurons); 
    Structure_delete (target);

    printf ("  Layer deleted\n");
}

---

// The network is simply a List of layers
typedef struct NeuralNetwork {
    List * layers;
} NeuralNetwork;




// Allocs the empty network and returns it
NeuralNetwork * NeuralNetwork_create (void) {
    NeuralNetwork * network;

    network = Structure_create (sizeof (NeuralNetwork));
    network->layers = List_create ();

    return network;
}


// Deletes the network and sets its pointer to NULL
void NeuralNetwork_delete (NeuralNetwork ** target) {
    if ((NULL == target) || (NULL == * target))
        return;

    printf ("Deleting network: %p\n", * target);

    // For each layer, delete layer
    for (size_t layerIndex = 0; layerIndex < List_size ((* target)->layers); layerIndex++) {
        NeuralLayer * buffer = List_get ((* target)->layers, layerIndex); // I'm using opaque structures
        NeuralLayer_delete (& buffer);
    }

    List_delete (& (* target)->layers);
    Structure_delete (target);

    printf ("Network deleted\n");
}


// We only need to know how many neurons the new layer will contain, they will have as many inputs as the number of neurons in the previous layer
void NeuralNetwork_addLayer (NeuralNetwork * dest, size_t neuronsCount) {
    if (NULL == dest->layers)
        List_add (& dest->layers, NeuralLayer_create (neuronsCount, 1));

    else {
        NeuralLayer * previousLayer = List_get (dest->layers, List_size (dest->layers) - 1);
        size_t inputsCount = NeuralLayer_getNeuronsCount (previousLayer);
        NeuralLayer * newLayer = NeuralLayer_create (neuronsCount, inputsCount);
        List_add (& dest->layers, newLayer);
    }
}

And my main:

int main (int argc, char * argv []) {
    NeuralNetwork * network = NeuralNetwork_create (); 
    NeuralNetwork_addLayer (network, 4); // First layer are inputs, one for each pixel
    NeuralNetwork_addLayer (network, 3); // Hidden layer of 15 neurons
    NeuralNetwork_addLayer (network, 2); // Output layer, 

    NeuralNetwork_delete (& network);
    return EXIT_SUCCESS;
}

Program crash during deallocation, NeuralNetwork_delete() calls NeuralLayer_delete() which calls Neuron_delete(), something is wrong with allocation / deallocation.

Oh, i forgot the base memory functions:

// Allocates memory, checks it and returns it
// Have to change the name, i don't use it only to alloc structures
void * Structure_create (size_t size) {
    void * structure;

    structure = malloc (size);
    if (NULL == structure) {
        printf ("Memory allocation failed, aborting.\n");
        system ("PAUSE");
        exit (EXIT_FAILURE);
    }

    return structure;
}



// Free the memory and sets the pointer to NULL
void Structure_delete (void ** target) {
    if ((NULL == target) || (NULL == * target))
        return;

    free (* target);
    * target = NULL;
}

What am I doing wrong ?

EDIT: Adding List_create / delete

typedef struct List {
    void * content;
    List * next;
    List * previous;
} List;




// Create a single item and inits it
static List * _List_createItem (void * content);



// Returns NULL to hide implementation
List * List_create (void) {
    return (List* )NULL;
}


void List_delete (List ** target) {
    if (NULL == target) {
        printf ("Tried to delete NULL list\n");
        return;
    }

    else if (NULL == * target)
        return;

    List_delete (& (* target)->next);
    Structure_delete (target); 
}


void * List_get (const List * const list, size_t index) {
    if (NULL == list) {
        printf ("Tried to get from List with index out of range\n");
        return NULL;
    }

    else if (0 == index) 
        return list->content;

    return List_get (list, index - 1);
}


// If the list was empty, it becomes a new list, so we need pointer to pointer
void List_add (List ** dest, void * content) {
    if (NULL == dest) {
        printf ("Tried to add item on NULL pointer\n");
        return;
    }

    else if ((NULL != * dest) && (NULL == (* dest)->next)) {
        (* dest)->next = _List_createItem (content);
        (* dest)->next->previous = * dest;
    }

    else if (NULL == * dest) 
        * dest = _List_createItem (content);

    else
        List_add (& (* dest)->next, content);
}


static List * _List_createItem (void * content) {    
    List * item;

    item = (List *)Structure_create (sizeof (List));
    item->content = content;
    item->next = NULL;
    item->previous = NULL;

    return item;
}

Answer 1

It seems that your List_get function isn't traversing the list properly:

return List_get (list, index - 1);

while it should be:

return List_get (list->next, index - 1);

Also, in NeuralLayer_delete function:

Structure_delete ((* target)->neurons);

should be:

Structure_delete (&(* target)->neurons);

---

As a side note: as you've already discovered this kind of "very manual" memory management is very brittle and error-prone. Consider transitioning to C++ in the future. It has RAII pattern and smart pointers for more robust resource management. It also provides dynamic array (vector) and linked list containers so you won't have to invent them (and make beginner mistakes :)) yourself. They are also likely to provide better performance.