[Asterisk-code-review] res smdi.c: Fix module ref counting and inverted test. (asterisk[master])

Corey Farrell asteriskteam at digium.com
Wed Oct 3 07:29:32 CDT 2018


Corey Farrell has posted comments on this change. ( https://gerrit.asterisk.org/10382 )

Change subject: res_smdi.c: Fix module ref counting and inverted test.
......................................................................


Patch Set 2: Code-Review+1

(2 comments)

https://gerrit.asterisk.org/#/c/10382/2/res/res_smdi.c
File res/res_smdi.c:

https://gerrit.asterisk.org/#/c/10382/2/res/res_smdi.c@a246
PS2, Line 246: 
In 2014 I replaced astobj with ao2 and failed to remove the invalid call to free here.  This was first released in 13.0.0 and nobody reported this crash.  Not sure if this means that it's impossible for interfaces to be cleaned or if it means that literally nobody uses this module.

https://reviewboard.asterisk.org/r/3758/


https://gerrit.asterisk.org/#/c/10382/2/res/res_smdi.c@249
PS2, Line 249: 		ast_module_unref(ast_module_info->self);
I think the interface/thread should not hold a reference to the module, we should just use ast_module_shutdown_ref in load_module.  This would allow graceful shutdown without allowing the module to be dlclose'd with active threads.

Honestly I don't care about this module and think it should be considered for deletion so feel free to ignore this comment.



-- 
To view, visit https://gerrit.asterisk.org/10382
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic288db51b58e395d6a2fc3847f77176c16988784
Gerrit-Change-Number: 10382
Gerrit-PatchSet: 2
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2 (1000185)
Gerrit-Comment-Date: Wed, 03 Oct 2018 12:29:32 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20181003/cb14c3f6/attachment-0001.html>


More information about the asterisk-code-review mailing list