Skip to content

Commit

Permalink
removing the un-aptly named cvector_free_each_and_free as per the dis…
Browse files Browse the repository at this point in the history
…cussion in #47.

Especially since now that we have a proper destructor, there's basically no need for this
macro (and, it really only saved one line of code)
  • Loading branch information
eteran committed Mar 13, 2024
1 parent 4c4023e commit 4dbe408
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 22 deletions.
14 changes: 0 additions & 14 deletions cvector_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,4 @@
} \
} while (0)

/**
* @brief cvector_free_each_and_free - calls `free_func` on each element
* contained in the vector and then destroys the vector itself
* @param vec - the vector
* @param free_func - function used to free each element in the vector with
* one parameter which is the element to be freed)
* @return void
*/
#define cvector_free_each_and_free(vec, free_func) \
do { \
cvector_for_each((vec), (free_func)); \
cvector_free(vec); \
} while (0)

#endif /* CVECTOR_UTILS_H_ */
3 changes: 2 additions & 1 deletion test.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ int main() {
cvector_vector_type(int) b = NULL;
cvector_vector_type(int) c = NULL;
cvector_vector_type(char *) str_vect = NULL;
cvector_set_elem_destructor(str_vect, free);

/* add some elements to the back */
cvector_push_back(v, 10);
Expand Down Expand Up @@ -156,6 +157,6 @@ int main() {
}
}

cvector_free_each_and_free(str_vect, free);
cvector_free(str_vect);
return 0;
}
20 changes: 13 additions & 7 deletions unit-tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ UTEST(test, vector_reserve) {
UTEST(test, vector_free_all) {
int i;
cvector_vector_type(char *) v = NULL;
cvector_set_elem_destructor(v, free);
for (i = 0; i < 10; ++i) {
char *p = malloc(6);
strcpy(p, "hello");
Expand All @@ -268,27 +269,32 @@ UTEST(test, vector_free_all) {
ASSERT_TRUE(cvector_size(v) == 10);
ASSERT_TRUE(cvector_capacity(v) >= 10);

cvector_free_each_and_free(v, free);
cvector_free(v);
}

UTEST(test, vector_for_each_int) {
cvector_iterator(char *) it;
cvector_iterator(int *) it;
int i;
cvector_vector_type(char *) v = NULL;
cvector_vector_type(int *) v = NULL;
cvector_set_elem_destructor(v, free);
for (i = 0; i < 10; ++i) {
char *p = malloc(6);
strcpy(p, "hello");
int *p = malloc(sizeof(int));
*p = 42;
cvector_push_back(v, p);
}

ASSERT_TRUE(cvector_size(v) == 10);
ASSERT_TRUE(cvector_capacity(v) >= 10);

cvector_for_each_in(it, v) {
ASSERT_TRUE(strcmp(*it, "hello") == 0);
/* NOTE(eteran): double pointer because we have an interator to an int*,
* so first deref to get the int*, the second to get int. Sure, this is a
* silly thing to do, but this is a test
*/
ASSERT_TRUE(**it == 42);
}

cvector_free_each_and_free(v, free);
cvector_free(v);
}

UTEST(test, vector_shrink_to_fit) {
Expand Down

5 comments on commit 4dbe408

@RobLoach
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not entirely sure why this is causing a heap fail

@eteran
Copy link
Owner Author

@eteran eteran commented on 4dbe408 Mar 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it? it passes all my tests

@eteran
Copy link
Owner Author

@eteran eteran commented on 4dbe408 Mar 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RobLoach oh wow, it is, replicated by running manually with --leak-check=full to valgrind, (passes locally without that flag). I'll investigate. Honestly, I kinda don't love the whole "element-destructor" thing. while it makes sense for C++, it's very un-C like

@eteran
Copy link
Owner Author

@eteran eteran commented on 4dbe408 Mar 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. is a no-op on non-null vectors. Will be fixing shortly.

@eteran
Copy link
Owner Author

@eteran eteran commented on 4dbe408 Mar 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

phew, fixed it. Honestly this has just made me dislike the destructors even more, LOL. I had forgotten how strange the semantics of it has to be to in order to work without templates.

Please sign in to comment.