Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(rax_tree): Introduce raxFreeWithCallback call in RaxTreeMap destructor #4255

Conversation

BagritsevichStepan
Copy link
Contributor

related to this comment

}

/* Free a whole radix tree with a callback */
void raxFreeWithCallback(rax *rax, void (*free_callback)(void*)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Niiiiice 💯

@BagritsevichStepan BagritsevichStepan force-pushed the df.search/rax-tree-add-raxFreeWithCallback branch from aebcbdc to c292bfe Compare December 4, 2024 16:40
@BagritsevichStepan BagritsevichStepan force-pushed the df.search/rax-tree-add-raxFreeWithCallback branch from c292bfe to 2f13e11 Compare December 4, 2024 17:37
@BagritsevichStepan
Copy link
Contributor Author

@BagritsevichStepan BagritsevichStepan requested review from BorysTheDev and chakaz and removed request for kostasrim December 10, 2024 12:50
rax_free(n);
rax->numnodes--;
}

/* Free the entire radix tree, invoking a free_callback function for each key's data.
* An additional argument is passed to the free_callback function.*/
void raxFreeWithCallbackAndArgument(rax *rax, void (*free_callback)(void*, void*), void* argument) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we call this:

Suggested change
void raxFreeWithCallbackAndArgument(rax *rax, void (*free_callback)(void*, void*), void* argument) {
void raxRecursiveFreeWithCallbackAndArgument(rax *rax, void (*free_callback)(void*, void*), void* argument) {

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so, because Redis provides raxFreeWithCallback as a public API. And raxRecursiveFree is an internal method and not part of the public API. So for consistency, I think raxFreeWithCallbackAndArgument would be a better choice.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok 🤷

rax_free(n);
rax->numnodes--;
}

/* Free the entire radix tree, invoking a free_callback function for each key's data.
* An additional argument is passed to the free_callback function.*/
void raxFreeWithCallbackAndArgument(rax *rax, void (*free_callback)(void*, void*), void* argument) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

ok 🤷

@BagritsevichStepan BagritsevichStepan merged commit 8d66c25 into dragonflydb:main Dec 23, 2024
10 checks passed
@BagritsevichStepan BagritsevichStepan deleted the df.search/rax-tree-add-raxFreeWithCallback branch December 23, 2024 04:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants