[Asterisk-code-review] funcs/func holdintercept: Actually add the HOLD INTERCEPT fu... (asterisk[master])

[Richard Mudgett]

Richard Mudgett has posted comments on this change.

Change subject: funcs/func_holdintercept: Actually add the HOLD_INTERCEPT function
......................................................................


Patch Set 2: Code-Review-1

(3 comments)

https://gerrit.asterisk.org/#/c/1454/2/funcs/func_holdintercept.c
File funcs/func_holdintercept.c:

Line 87: 		ast_log(AST_LOG_WARNING, "Cannot remove HOLD_INTERCEPT from %s: HOLD_INTERCEPT not currently enabled\n",
       : 		        ast_channel_name(chan));
This maybe should be just a debug message as the warning could be considered a nuisance.


Line 120: 	if (frame_type != AST_CONTROL_HOLD && frame_type != AST_CONTROL_UNHOLD) {
You should be checking that f->frametype != AST_FRAME_CONTROL before checking if f->subclass.integer is a member of enum ast_control_frame_type.


Line 156: 		ast_log(AST_LOG_WARNING, "HOLD_INTERCEPT already set on '%s'\n",
        : 		        ast_channel_name(chan));
This likely should be a debug message.  The warning message could be considered a nuisance.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ida38278f259dd07c334a36f9b7d5475b5db72396
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes