-
Notifications
You must be signed in to change notification settings - Fork 48
Plug memory leak #375
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
base: main
Are you sure you want to change the base?
Plug memory leak #375
Conversation
PR Verification: ❌ failure |
void | ||
_DML_free_qname_cache(qname_cache_t *cache) | ||
{ | ||
for (int i = 0; i < 4; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this magic 4 be a define?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sizeof(cache->bufs)/sizeof(cache->bufs[0])
is also a pretty common C pattern (though typically broken out to a macro)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That works for me too, I wasn't sure if cache->bufs
was an array or pointer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a potential pitfall, here; model code can be called following _destroy
due to immediate after (see c_backend.generate_deinit
,) and that model code could call _qname
and reallocate the buffers.
The surefire way to address that is to free the qname cache separately from _destroy
. Say, introduce the method _destroy_qname_cache
and then codegen_inline_byname
it in generate_deinit
after the final _DML_execute_immediate_afters_now
. Or we could just be fine with that extremely unlikely leak.
void | ||
_DML_free_qname_cache(qname_cache_t *cache) | ||
{ | ||
for (int i = 0; i < 4; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sizeof(cache->bufs)/sizeof(cache->bufs[0])
is also a pretty common C pattern (though typically broken out to a macro)
method _destroy() { _rec_destroy(); } | ||
method _destroy() { | ||
_rec_destroy(); | ||
_DML_free_qname_cache(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing its argument.
No description provided.