[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 4: Code-Review-1

(11 comments)

https://gerrit.asterisk.org/#/c/1884/4//COMMIT_MSG
Commit Message:

Line 14: The ast_vm_register and ast_vm_greeter_register functions in app.c
       : were modified to return AST_MODULE_LOAD_SKIP if there was already
       : a voicemail module registered instead of -1. The modules' load_module
       : functions were then modified to check for SKIP and return SKIP
       : instead of the -1 in response to the load.  Since the modules were
       : originally returning the -1 on load, it was being interpreted as
       : AST_MODULE_LOAD_FAILURE which was causing Asterisk to stop so this
       : needed to be cleaned up anyway.
This paragraph needs updating.


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

Line 41:    app_voicemail will be skipped.  Use 'preload=app_voicemail.so' in
...will decline to load.  Use...


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

Line 14702:  * Module loading including tests for configuration or dependencies.
          :  * This function can return AST_MODULE_LOAD_FAILURE, AST_MODULE_LOAD_DECLINE,
          :  * AST_MODULE_LOAD_SKIP or AST_MODULE_LOAD_SUCCESS.
          :  *
          :  * If a dependency or environment variable fails tests return AST_MODULE_LOAD_FAILURE.
          :  *
          :  * If the module can not load the configuration file or other non-critical problem return
          :  * AST_MODULE_LOAD_DECLINE.
          :  *
          :  * If another module already registered for voicemail or greeter, return AST_MODULE_LOAD_SKIP.
          :  *
Needs updating.


Line 14761: 	/* ast_vm_register may return SKIP if another module registered for vm */
SKIP -> DECLINE


Line 14764: 		if (res  == AST_MODULE_LOAD_DECLINE) {
          : 			ast_log(LOG_WARNING, "Skipping.  Another voicemail provider is already registered\n");
          : 		} else {
          : 			ast_log(LOG_ERROR, "Failure registering as a voicemail provider\n");
          : 		}
These messages are redundant as ast_vm_register() has already gripped.


Line 14773: 	/* ast_vm_greeter_register may return SKIP if another module registered as a greeter */
same


Line 14776: 		if (res  == AST_MODULE_LOAD_DECLINE) {
          : 			ast_log(LOG_WARNING, "Skipping.  Another voicemail greeter is already registered\n");
          : 		} else {
          : 			ast_log(LOG_ERROR, "Failure registering as a greeter provider\n");
          : 		}
These messages are redundant as ast_vm_greeter_register() has already gripped.


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

Line 583:  * \retval 1 (AST_MODULE_LOAD_DECLINE) if there's already another provider registered.
This file could be reverted as both functions have already gripped about what is wrong.  The callers don't need to distinguish as to why any more since they do the same thing either way.


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

Line 497: 		return AST_MODULE_LOAD_DECLINE;
This file could be reverted as both functions already gripe about what is wrong.  The callers don't need to distinguish as to why any more since they do the same thing either way.


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

Line 949: 	/* ast_vm_register may return SKIP if another module registered for vm */
SKIP -> DECLINE


Line 952: 		if (res  == AST_MODULE_LOAD_DECLINE) {
        : 			ast_log(LOG_WARNING, "Skipping.  Another voicemail provider is already registered\n");
        : 		} else {
        : 			ast_log(LOG_ERROR, "Failure registering as a voicemail provider\n");
        : 		}
These messages are redundant as ast_vm_register() has already gripped.

Actually the changes in this function could be reverted.


-- 
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: 4
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