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

Question

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;
}

Answer 1

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; 
}

Answer 2

You have a problem here:

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; 
}

This function is called twice in main(), but returns a pointer to a static variable, meaning it will return the same address every time it is called. So when main() appears to create pointers to two different lists, they are actually pointers to a single list, namely static list res.

Answer 3

There are multiple problems in your code:

• hiding the implementation from the user does not require an indirect as posted, you can just declare the struct list without a definition.

• create_list returns the address of a static object, no surprise freeing this has undefined behavior, not to mention the fact that all list objects objects are actually the same object.

• the line int n = (self)->capacity; at the beginning of the switch block in _alloc() is never executed. Move this line before the switch statement.

• you should not use identifiers starting with a _.

• parenthesizing (self) is useless and confusing.

Here is a simplified version:

primitive_list.h:

#ifndef PRIMITIVE_LIST_H
#define PRIMITIVE_LIST_H

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

typedef struct list list;

list *list_create(list_type type);
int list_push(list *self, ...);
void list_pop(list *self);
int list_get(const list *self, int index, ...);
int list_set(list *self, int index, ...);
int list_empty(const list *self);
int list_length(const list *self);
void list_print(const char *msg, const list *self);
void list_free(list **self);
#endif

primitive_list.c:

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

struct list {
    list_type type;
    int capacity;
    int length;
    void *arr;
};

list *list_create(list_type type) {
    list *res = malloc(sizeof(*res));
    if (res) {
        res->type = type;
        res->capacity = 0;
        res->length = 0;
        res->arr = NULL;
    } else {
        fprintf(stderr, "list_create: out of memory!\n");
    }
    return res;
}

static int list_element_size(list_type type) {
    switch (type) {
    case INT:       return sizeof(int);
    case LONG_INT:  return sizeof(long long);
    case DECIMAL:   return sizeof(double);
    case CHAR:      return sizeof(char);
    }
    return 0;
}

int list_push(list *self, ...) {
    va_list arg;

    if (self->length == self->capacity) {
        if (self->capacity == 0) {
            self->capacity = 1;
            self->arr = calloc(self->capacity, list_element_size(self->type));
            if (self->arr == NULL) {
                fprintf(stderr, "list_push: out of memory!\n");
                return 0;
            }
        } else {
            size_t size = self->capacity * list_element_size(self->type);
            void *new_arr = realloc(self->arr, size * 2);
            if (new_arr == NULL) {
                fprintf(stderr, "list_push: out of memory!\n");
                return 0;
            }
            memset((char *)new_arr + size, 0, size);
            self->arr = new_arr;
            self->capacity *= 2;
        }
    }

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

void list_pop(list *self) {
    if (self->length == 0) {
        fprintf(stderr, "list_pop: list is empty!\n");
    } else {
        self->length--;
    }
}

int list_get(const list *self, int index, ...) {
    int rc = 0;
    if (index >= 0 && index < self->length) {
        va_list arg;
        va_start(arg, index);
        switch (self->type) {
        case INT:
            *va_arg(arg, int *) = ((int *)self->arr)[index];
            rc = 1;
            break;
        case LONG_INT:
            *va_arg(arg, long long *) = ((long long *)self->arr)[index];
            rc = 1;
            break;
        case DECIMAL:
            *va_arg(arg, double *) = ((double *)self->arr)[index];
            rc = 1;
            break;
        case CHAR:
            *va_arg(arg, char *) = ((char *)self->arr)[index];
            rc = 1;
            break;
        }
        va_end(arg);
    }
    return rc;
}

int list_set(list *self, int index, ...) {
    int rc = 0;
    if (index >= 0 && index < self->length) {
        va_list arg;
        va_start(arg, index);
        switch (self->type) {
        case INT:
            ((int *)self->arr)[index] = va_arg(arg, int);
            rc = 1;
            break;
        case LONG_INT:
            ((long long *)self->arr)[index] = va_arg(arg, long long);
            rc = 1;
            break;
        case DECIMAL:
            ((double *)self->arr)[index] = va_arg(arg, double);
            rc = 1;
            break;
        case CHAR:
            ((char *)self->arr)[index] = (char)va_arg(arg, int);
            rc = 1;
            break;
        }
        va_end(arg);
    }
    return rc;
}

int list_empty(const list *self) {
    return self->length == 0;
}

int list_length(const list *self) {
    return self->length;
}

void list_print(const char *msg, const list *self) {
    if (msg) {
        printf("%s: ", msg);
    }
    for (int i = 0; i < self->length; 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("%f ", ((double *)self->arr)[i]);
            break;
        case CHAR:
            printf("%c ", ((char *)self->arr)[i]);
            break;
        }
    }
    printf("\n");
}

void list_free(list **self) {
    if (*self) {
        free((*self)->arr);
        free(*self);
        *self = NULL;
    }
}

test.c:

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

int main(int argc, char **argv) {
    //Decimal List
    list *nums = list_create(DECIMAL);
    list_push(nums, 3.14159);
    list_push(nums, 6.25);
    list_push(nums, 22.2222);
    list_push(nums, 100.0);

    //Character list
    list *chars = list_create(CHAR);
    list_push(chars, 'A');
    list_push(chars, 'w');
    list_push(chars, 'Z');
    list_push(chars, 'q');
    list_push(chars, 'P');

    // print the lists
    list_print("nums", nums);
    list_print("chars", chars);

    double x;
    char c;
    if (list_get(nums, 2, &x))
        printf("nums[2] = %f\n", x);
    if (list_get(chars, 2, &c))
        printf("chars[2] = '%c'\n", c);
    if (list_set(nums, 3, 123.45))
        printf("set nums[3] to %f\n", 123.45);
    if (list_set(chars, 3, '*'))
        printf("set chars[3] to '%c'\n", '*');

    // print the lists
    list_print("nums", nums);
    list_print("chars", chars);

    // free the lists
    list_free(&nums);
    list_free(&chars);
    return 0;
}

Output:

nums: 3.141590 6.250000 22.222200 100.000000
chars: A w Z q P
nums[2] = 22.222200
chars[2] = 'Z'
set nums[3] to 123.450000
set chars[3] to '*'
nums: 3.141590 6.250000 22.222200 123.450000
chars: A w Z * P