Search code examples
arrayscdynamiccontainersgeneric-list

Programming crashing at the end, when deleting the resources. Implementation of pseudo-generic dynamic array list in C


I have implemented a pseudo-generic dynamic array in C, for primitive types. It takes enum constants as identifier for each type- INT, LONG_INT, DECIMAL, CHAR. The main struct list has a member _assets, which is a void pointer (to have a level of abstraction). This void pointer is type-casted to another struct (which is the main internal working of the container) in the C file implementation. I tested the list with all the types, and it worked perfectly fine. The problem arose when I created 2 lists, performed some operations, and then deleted the both lists together in the same order they were created. It gives me an error:- Critical error detected c0000374; and says, unable to open 'free-base.cpp'. The deletion takes play through the free_assets method. The 2 lists work perfectly fine when I delete the first list object before using the second list object, which is quiet unusual, since they are 2 different objects.

primitive_list.h

#ifndef PRIMITIVE_LIST_H 
#define PRIMITIVE_LIST_H 

typedef enum { INT, LONG_INT, DECIMAL, CHAR } list_types; 

typedef struct 
{
    void *_assets;
} list; 

list *create_list(list_types type);
void push(list **self, ...); 
void pop(list **self); 
void*at(list *self, const int index); 
int empty(list *self); 
unsigned length(list *self);
void print_list(list *self);
void free_assets(list **self);
#endif 

primitive_list.c

#include <stdio.h>
#include <stdlib.h> 
#include <stdarg.h>
#include "primitive_list.h"

typedef struct 
{
    int capacity; 
    int size; 
    void *arr; 
    list_types type; 
    int empty; 
} _list;

_list _create_list(list_types type)
{
    _list res; 
    res.type = type; 
    res.capacity = 1; 
    res.size = 0; 
    res.empty = 1; 
    return res;
}

void _alloc(_list *self)
{
    switch ((self)->type)
    {
        int n = (self)->capacity;
        case INT:
            (self)->arr = calloc(n, sizeof(int)); 
            break; 
        case LONG_INT:
            (self)->arr = calloc(n, sizeof(long long)); 
            break;
        case DECIMAL:
            (self)->arr = calloc(n, sizeof(double));
            break; 
        case CHAR:
            (self)->arr = calloc(n, sizeof(char));
            break;
    }
    return;
}

void _realloc(_list *self, size_t Buffer_size)
{
    (self)->capacity = Buffer_size; 
    list_types type = (self)->type;
    int n = (self)->capacity; 
    int s = (self)->size;
    if (type == INT) {
        int *new_array = (int *)calloc(n, sizeof(int));
        for (size_t i = 0; i < s; i++)
        {
            new_array[i] = ((int *)(self)->arr)[i];
        }
        free((self)->arr); 
        (self)->arr = (void *)new_array;
    } else if (type == LONG_INT) {
        long long *new_array = (long long *)calloc(n, sizeof(long long));
        for (size_t i = 0; i < s; i++)
        {
            new_array[i] = ((long long *)(self)->arr)[i];
        }
        free((self)->arr); 
        (self)->arr = (void *)new_array;
    } else if (type == DECIMAL) {
        double *new_array = (double *)calloc(n, sizeof(double));
        for (size_t i = 0; i < s; i++)
        {
            new_array[i] = ((double *)(self)->arr)[i];
        }
        free((self)->arr); 
        (self)->arr = (void *)new_array;
    } else if (type == CHAR) {
        char *new_array = (char *)calloc(n, sizeof(char));
        for (size_t i = 0; i < s; i++)
        {
            new_array[i] = ((char *)(self)->arr)[i];
        }
        free((self)->arr); 
        (self)->arr = (void *)new_array;
    }
    return;
}

void _push(_list *self, ...)
{
    if ((self)->empty)
    {
        (self)->empty = 0; 
        _alloc(self); 
    }

    if ((self)->size == (self)->capacity)
        _realloc(self, (self)->capacity * 2); 
    
    va_list arg; 
    va_start(arg, self); 
    switch ((self)->type)
    {
        case INT:
            ((int *)(self)->arr)[(self)->size] = va_arg(arg, int);
            break; 
        case LONG_INT:
            ((long long *)(self)->arr)[(self)->size] = va_arg(arg, long long);
            break; 
        case DECIMAL:
            ((double *)(self)->arr)[(self)->size] = va_arg(arg, double);
            break; 
        case CHAR:
            ((char *)(self)->arr)[(self)->size] =(char)va_arg(arg, int);
            break; 
    }
    (self)->size++;
    va_end(arg); 
    return;
}

void _pop(_list *self)
{
    if ((self)->empty)
    {
        fprintf(stderr,"List is empty!\n"); 
        return;
    }
    (self)->size--;
    return;
}

void *_at(_list *self, const int index)
{
    void *res; 
    switch ((self)->type)
    {
        case INT:
            res = malloc(sizeof(int)); 
            *((int *)res) = ((int *)(self)->arr)[index];
            break; 
        case LONG_INT:
            res = malloc(sizeof(long long)); 
            *((long long *)res) = ((long long *)(self)->arr)[index];
            break;
        case DECIMAL:
            res = malloc(sizeof(double)); 
            *((double *)res) = ((double *)(self)->arr)[index];
            break; 
        case CHAR:
            res = malloc(sizeof(char)); 
            *((char *)res) = ((char *)(self)->arr)[index];
            break;
    }
    return res;
}

int _empty(_list *self)
{
    return self->empty;
}

unsigned _length(_list *self)
{
    return self->size;
}

void _print_list(_list *self)
{
    for (size_t i = 0; i < self->size; i++)
    {
        switch(self->type)
        {
            case INT:
                printf("%d ", ((int *)self->arr)[i]);
                break;
            case LONG_INT:
                printf("%lld ", ((long long *)self->arr)[i]);
                break;
            case DECIMAL:
                printf("%lf ", ((double *)self->arr)[i]);
                break;
            case CHAR:
                printf("%c ",((char*)self->arr)[i]);
                break;
        }
    }
    printf("\n");
    return;
}

void _free_list(_list *self)
{
    free((self)->arr);
}

list *create_list(list_types type)
{
    static list res; 
    _list obj = _create_list(type); 
    res._assets = malloc(sizeof(_list)); 
    *((_list*)res._assets) = obj;
    return &res; 
}

void push(list **self, ...)
{
    va_list arg; 
    va_start(arg, self); 
    
    switch (((_list *)(*self)->_assets)->type)
    {
        case INT:
            _push(((_list *)(*self)->_assets), va_arg(arg, int));
            break;
        case LONG_INT:
            _push(((_list *)(*self)->_assets), va_arg(arg, long long));
            break; 
        case DECIMAL:
            _push(((_list *)(*self)->_assets), va_arg(arg, double));
            break; 
        case CHAR:
            _push(((_list *)(*self)->_assets), (char)va_arg(arg, int));
            break; 
    }
    va_end(arg); 
    return;
}

void pop(list **self)
{
    _pop(((_list *)(*self)->_assets));
    return;
}

void *at(list *self, const int index)
{
    return _at((_list *)self->_assets, index);
}

int empty(list *self)
{
    return _empty((_list *)self->_assets);
}

unsigned length(list *self)
{
    return _length((_list *)self->_assets);
}

void print_list(list *self)
{
    _print_list((_list *)self->_assets);
    return;
}

void free_assets(list **self)
{
    _free_list(((_list *)(*self)->_assets)); 
    free((*self)->_assets);
}

test.c

#include <stdio.h> 
#include <stdlib.h> 
#include "primitive_list.h"

int main(int argc, char **argv)
{
    //Decimal List 
    list *nums = create_list(DECIMAL); 
    push(&nums, 3.14159); 
    push(&nums, 6.25); 
    push(&nums, 22.2222); 
    push(&nums, 100.0);
    print_list(nums); 
    
    //Character list 
    list *chars = create_list(CHAR); 
    push(&chars, 'A');
    push(&chars, 'w');
    push(&chars, 'Z');
    push(&chars, 'q');
    push(&chars, 'P');
    print_list(chars); 
    
    //Code causing the error
    free_assets(&nums); 
    free_assets(&chars);
    return 0;
}

Solution

  • Your code here is a bit confusing, as it appears you're burying your underlying structure in another structure for no reason. There's no reason you cannot simply (if you want to keep implementation details private) write typedef struct list list; in the header and in your implementation have

    struct list {
        int capacity; 
        int size; 
        void *arr; 
        list_types type; 
        int empty;
    };
    

    This way, there would be much less confusion about having to deal with the _assets pointer; you could simply address the list members directly without a dereference and cast (this is needlessly confusing).

    Your main issue here is actually due to bad memory management in your create_list function due to its implementation. Specifically, these two lines:

    list *create_list(list_types type)
    {
        static list res; 
        
        /* ... */
        
        return &res; 
    }
    

    What the static keyword does here is to define a global variable shared between all invocations of this function. That is, every time you call create_list, the res variable is set to the same value as it was at the last return from the function. This is why you can return &res from this function to get a valid pointer outside the function: the res variable is stored in global memory for your program instead of in the function's stack frame.

    There are then (at least!) two issues with this program:

    1. The pointer to every list you create will be the same, so every list created will override the memory of the last list, which not only leaks memory (the pointer to the _assets structure), but makes only the last created list valid. In fact, your nums and chars lists should have exactly the same value ones chars is created.
    2. You're calling free on memory you didn't allocate with malloc. Because you return the address of a bona-fide global variable from your create_list function, you're returning a pointer to memory created by the Operating System when your program is first loaded. You DO NOT own this memory, and attempting to call free on it has exactly the effect you're seeing here: it will crash your program.

    Both of these issues can be resolved in the same way: properly manage your memory. Generally, if you want to create a structure in memory in C, you should be calling malloc with the proper size, initializing the value of the new memory, and returning the pointer to it back to the caller. This can be implemented in your case as:

    list *create_list(list_types type)
    {
        list res = malloc(sizeof(list));
        _list obj=_create_list(type); 
        res._assets=malloc(sizeof(_list)); 
        *((_list *)res._assets)=obj;
        return res; 
    }
    

    Or, if you change the layout of list as I suggest above, simply:

    list *create_list(list_types type)
    {
        list res = malloc(sizeof(list); 
        res->type = type; 
        res->capacity = 1; 
        // ...
        return res; 
    }