eteran / c-vector

A dynamic array implementation in C similar to the one found in standard C++
MIT License
737 stars 109 forks source link

Feature: element destructor #37

Closed julianhartmer closed 2 years ago

julianhartmer commented 2 years ago

Add function pointer that mimcs the element's destructor. The vector may be used with complex data structures that hold dynamically allocated memory. To prevent leaking this memory when elements are removed, I introduce a new field at the start of the cvector meta-data called elem_destructor(). Each time a element is removed, the function is called to prevent memory leaks. Initially, the function pointer is set to NULL. If that's the case, then no destructor is called, ensuring backwards compatibility.

This PR also already contains the commits from PR 36, so please land this one first.

Here is the test I wrote to check the functionality:

jh@jh-ThinkPad-X250:~/GameDev/c-vector
$ cat test_changes.c
#include <stdio.h>
#include "cvector.h"

struct complex_struct {
    char *allocated_string;
    int i;
};

void free_fn(void *elem)
{
    struct complex_struct *a = elem;
    free(a->allocated_string);
}

int main() {
    cvector_vector_type(struct complex_struct) vec = NULL;

    struct complex_struct a = {
        .i = 10
    };

    a.allocated_string = strdup("hello World");
    cvector_push_back(vec, a);
    cvector_set_elem_destructor(vec, free_fn);

    a.allocated_string = strdup("hello github");
    a.i++;
    cvector_push_back(vec, a);

    printf("%d: %s\n", vec[0].i, vec[0].allocated_string);
    printf("%d: %s\n", vec[1].i, vec[1].allocated_string);

    cvector_pop_back(vec);
    cvector_free(vec);
}
jh@jh-ThinkPad-X250:~/GameDev/c-vector
$ gcc test_changes.c -g -Wall -Wextra -pedantic && valgrind ./a.out
==12953== Memcheck, a memory error detector
==12953== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==12953== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==12953== Command: ./a.out
==12953== 
10: hello World
11: hello github
==12953== 
==12953== HEAP SUMMARY:
==12953==     in use at exit: 0 bytes in 0 blocks
==12953==   total heap usage: 5 allocs, 5 frees, 1,145 bytes allocated
==12953== 
==12953== All heap blocks were freed -- no leaks are possible
==12953== 
==12953== For lists of detected and suppressed errors, rerun with: -s
==12953== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
eteran commented 2 years ago

Interesting. This could also effectively deprecate the cvector_free_each_and_free function. I'll take a look at it :-)

julianhartmer commented 2 years ago

Thanks for your feedback, I updated the commit.

julianhartmer commented 2 years ago

Now, the formatting is correct again

eteran commented 2 years ago

I added one more comment but otherwise is looking good!

julianhartmer commented 2 years ago

I accidentally pushed my changes to fix alignment issues on this branch. Reverted the commit immediately. I think it's best to keep this two issues seperate to prevent this PR from becoming a monster.

eteran commented 2 years ago

OK, I think this is looking pretty good!

I may decide to make a follow up PR myself to make the destructor feature opt in/out, but for now this is looking like it "just works" 👍🏻