add unregister methods to luanetfilter, luaxdp, luahid #411
Conversation
Eshaan-byte
force-pushed
the
add-unregister-methods
branch
from
8753099 to
39d2c7d
Compare
| luahid_delete(hid); | ||
|
|
||
| lunatik_object_t *runtime = hid->runtime; | ||
| if (runtime != NULL) { |
There was a problem hiding this comment.
please, don't mix cosmetic changes on new feat and fixes.. if you want to suggest changes on our code style, you are welcome to do so, but do it as a separate PR.
There was a problem hiding this comment.
You're right, sorry about that. I will revert it. I'll keep style changes separate in the future.
There was a problem hiding this comment.
please, let me know then I can give it another review round.. thanks..
There was a problem hiding this comment.
Done! Reverted the cosmetic changes and kept the original != NULL style. Ready for another review round.
lib/luanetfilter.c
Outdated
| nf->runtime = NULL; | ||
| } | ||
|
|
||
| static void luanetfilter_release(void *private) |
There was a problem hiding this comment.
why having this indirection?
There was a problem hiding this comment.
The helper avoids duplicating the cleanup logic in both release() and unregister(). I followed the same pattern from luadevice_delete() which is called from both luadevice_stop() and luadevice_release().
Happy to change it if you have a different approach in mind!
There was a problem hiding this comment.
it's not the same, right? we don't simply call one in the other on luadevice.. in this case here, I think we should just call the nested function directly instead of having a indirect call.
There was a problem hiding this comment.
You're right! I removed the helper function and inlined the cleanup code directly in both release() and unregister(). Changes pushed.
Adds explicit unregister() methods following the pattern from luadevice_stop() and luanotifier_stop(). - luanetfilter: adds unregister() and refactors cleanup into helper - luaxdp: adds unregister as alias for detach() - luahid: adds unregister() and refactors cleanup into helper All use lunatik_unregisterobject() for explicit cleanup. Fixes luainkernel#403
Eshaan-byte
force-pushed
the
add-unregister-methods
branch
from
39d2c7d to
f044600
Compare
| lunatik_object_t *object = lunatik_checkobject(L, 1); | ||
| luahid_t *hid = (luahid_t *)object->private; | ||
|
|
||
| lunatik_lock(object); |
There was a problem hiding this comment.
Did you test it? It looks like a deadlock.
2 participants
Adds explicit unregister() methods following the pattern from luadevice_stop() and luanotifier_stop().
All use lunatik_unregisterobject() for explicit cleanup.
Fixes #403