[Asterisk-code-review] voicemail: Move app voicemail / res mwi external conflict to... (asterisk[13])

[Richard Mudgett]

Richard Mudgett has posted comments on this change.

Change subject: voicemail: Move app_voicemail / res_mwi_external conflict to runtime
......................................................................


Patch Set 2: Code-Review-1

(11 comments)

In loader.c:
	case AST_MODULE_LOAD_SKIP: /* modules should never return this value */


You should use AST_MODULE_LOAD_DECLINE instead.

https://gerrit.asterisk.org/#/c/1884/2/CHANGES
File CHANGES:

Line 40: Voicemail
       : ------------------
       :  * app_voicemail and res_mwi_external can now be built together.  The default
       :    remains to build app_voicemail and not res_mwi_external but if they are
       :    both built, the load order will cause res_mwi_external to load first and
       :    app_voicemail will be skipped.  Use 'preload=app_voicemail.so' in
       :    modules.conf to force app_voicemail to be the voicemail provider.
This needs to go into the 13.8.0 section since 13.7.0-rc has already been created.


https://gerrit.asterisk.org/#/c/1884/2/apps/app_voicemail.c
File apps/app_voicemail.c:

Line 14734: 		return res;
This probably should return AST_MODULE_LOAD_DECLINE instead of any old non-zero value.  In this case the non-zero value would be -1 for a malloc failure.


Line 14766: 		return res;
just return AST_MODULE_LOAD_SKIP


Line 14770: 		return res;
just return AST_MODULE_LOAD_DECLINE instead of -1 (FAILURE)


Line 14778: 		return res;
Just return AST_MODULE_LOAD_SKIP


Line 14782: 		return res;
just return AST_MODULE_LOAD_DECLINE instead of -1 (FAILURE)


https://gerrit.asterisk.org/#/c/1884/2/include/asterisk/app.h
File include/asterisk/app.h:

Line 583:  * \retval -2 (AST_MODULE_LOAD_SKIP) if there's already another provider registered.
AST_MODULE_LOAD_SKIP = 2 not -2


Line 652:  * \retval -2 (AST_MODULE_LOAD_SKIP) if there's already another greeter registered.
same


https://gerrit.asterisk.org/#/c/1884/2/main/app.c
File main/app.c:

Line 497: 		return AST_MODULE_LOAD_SKIP ;
extra space before ;


https://gerrit.asterisk.org/#/c/1884/2/res/res_mwi_external.c
File res/res_mwi_external.c:

Line 954: 		return res;
just return AST_MODULE_LOAD_SKIP


Line 958: 		return res;
just return AST_MODULE_LOAD_DECLINE instead of -1 (FAILURE)


-- 
To view, visit https://gerrit.asterisk.org/1884
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7d98d4e8a3b87b8df9e51c2608f0da6ddfb89247
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes