[Asterisk-code-review] Remove redundant module checks and references. (asterisk[master])

[Richard Mudgett]

Richard Mudgett has posted comments on this change. ( https://gerrit.asterisk.org/7989 )

Change subject: Remove redundant module checks and references.
......................................................................


Patch Set 4: Code-Review-1

(12 comments)

https://gerrit.asterisk.org/#/c/7989/4/apps/app_confbridge.c
File apps/app_confbridge.c:

https://gerrit.asterisk.org/#/c/7989/4/apps/app_confbridge.c@2389
PS4, Line 2389: 	if (ast_test_flag(&user.u_profile, USER_OPT_JITTERBUFFER)) {
              : 		ast_func_write(chan, "JITTERBUFFER(adaptive)", "default");
              : 	}
              : 
              : 	if (ast_test_flag(&user.u_profile, USER_OPT_DENOISE)) {
These checks are still needed.  They are not to prevent loading because a dependency is missing.  They are to see if the optional functionality is available to use.

On the other hand, ast_func_write() logs an error if you are trying to use the dialplan functions but didn't install the modules.  Not loading the functionality but wanting to use it is more of a configuration error anyway.

We have certainly done our best now to ensure that they are loaded and running before app_confbridge starts and that they will remain available while app_confbridge is running.


https://gerrit.asterisk.org/#/c/7989/4/apps/app_meetme.c
File apps/app_meetme.c:

https://gerrit.asterisk.org/#/c/7989/4/apps/app_meetme.c@3591
PS4, Line 3591: 	if (!ast_test_flag64(confflags, CONFFLAG_DONT_DENOISE)) {
Same here as in app_confbridge.


https://gerrit.asterisk.org/#/c/7989/4/res/res_hep_pjsip.c
File res/res_hep_pjsip.c:

https://gerrit.asterisk.org/#/c/7989/4/res/res_hep_pjsip.c@235
PS4, Line 235: 	if (!hepv3_is_loaded()) {
             : 		ast_log(AST_LOG_WARNING, "res_hep is disabled; declining module load\n");
             : 		return AST_MODULE_LOAD_DECLINE;
             : 	}
This shouldn't be needed if res_hep declines to load when it is disabled.

In res_hep.c:load_module()

Replace:

if (aco_process_config(&cfg_info, 0) == ACO_PROCESS_ERROR) {
	goto error;
}


With:

if (aco_process_config(&cfg_info, 0) == ACO_PROCESS_ERROR
	|| !hepv3_is_loaded()) {
	goto error;
}


https://gerrit.asterisk.org/#/c/7989/4/res/res_hep_rtcp.c
File res/res_hep_rtcp.c:

https://gerrit.asterisk.org/#/c/7989/4/res/res_hep_rtcp.c@160
PS4, Line 160: 	if (!hepv3_is_loaded()) {
             : 		ast_log(AST_LOG_WARNING, "res_hep is disabled; declining module load\n");
             : 		return AST_MODULE_LOAD_DECLINE;
             : 	}
Same as res_hep_pjsip.c


https://gerrit.asterisk.org/#/c/7989/4/res/res_pjsip_diversion.c
File res/res_pjsip_diversion.c:

https://gerrit.asterisk.org/#/c/7989/4/res/res_pjsip_diversion.c@429
PS4, Line 429: 	.requires = "res_pjsip",
Include res_pjsip_session too since this module used CHECK_PJSIP_SESSION_MODULE_LOADED().  Unless you are assuming the res_pjsip_session module has already been merged into res_pjsip.so.


https://gerrit.asterisk.org/#/c/7989/4/res/res_pjsip_dtmf_info.c
File res/res_pjsip_dtmf_info.c:

https://gerrit.asterisk.org/#/c/7989/4/res/res_pjsip_dtmf_info.c@178
PS4, Line 178: 	.requires = "res_pjsip",
Same here about res_pjsip_session as in res_pjsip_diversion.


https://gerrit.asterisk.org/#/c/7989/4/res/res_pjsip_empty_info.c
File res/res_pjsip_empty_info.c:

https://gerrit.asterisk.org/#/c/7989/4/res/res_pjsip_empty_info.c@84
PS4, Line 84: 	.requires = "res_pjsip",
Same here about res_pjsip_session as in res_pjsip_diversion.


https://gerrit.asterisk.org/#/c/7989/4/res/res_pjsip_messaging.c
File res/res_pjsip_messaging.c:

https://gerrit.asterisk.org/#/c/7989/4/res/res_pjsip_messaging.c@850
PS4, Line 850: 	.requires = "res_pjsip",
Same here about res_pjsip_session as in res_pjsip_diversion.


https://gerrit.asterisk.org/#/c/7989/4/res/res_pjsip_nat.c
File res/res_pjsip_nat.c:

https://gerrit.asterisk.org/#/c/7989/4/res/res_pjsip_nat.c@375
PS4, Line 375: 	.requires = "res_pjsip",
Same here about res_pjsip_session as in res_pjsip_diversion.


https://gerrit.asterisk.org/#/c/7989/4/res/res_pjsip_one_touch_record_info.c
File res/res_pjsip_one_touch_record_info.c:

https://gerrit.asterisk.org/#/c/7989/4/res/res_pjsip_one_touch_record_info.c@126
PS4, Line 126: 	.requires = "res_pjsip",
Same here about res_pjsip_session as in res_pjsip_diversion.


https://gerrit.asterisk.org/#/c/7989/4/res/res_pjsip_path.c
File res/res_pjsip_path.c:

https://gerrit.asterisk.org/#/c/7989/4/res/res_pjsip_path.c@259
PS4, Line 259: 	.requires = "res_pjsip",
Same here about res_pjsip_session as in res_pjsip_diversion.


https://gerrit.asterisk.org/#/c/7989/4/res/res_pjsip_rfc3326.c
File res/res_pjsip_rfc3326.c:

https://gerrit.asterisk.org/#/c/7989/4/res/res_pjsip_rfc3326.c@156
PS4, Line 156: 	.requires = "res_pjsip",
Same here about res_pjsip_session as in res_pjsip_diversion.



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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I332a6e8383d4c72c8e89d988a184ab8320c4872e
Gerrit-Change-Number: 7989
Gerrit-PatchSet: 4
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Sun, 21 Jan 2018 17:28:25 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180121/db1bcb77/attachment-0001.html>