[Home]

Summary:ASTERISK-11032: [patch] Check the value of the 'penalty' parameter for the AMI action 'QueuePenalty'
Reporter:Tomás Laureano Peralta Tormey (laureano)Labels:
Date Opened:2007-12-13 17:24:27.000-0600Date Closed:2007-12-19 17:14:30.000-0600
Priority:TrivialRegression?No
Status:Closed/CompleteComponents:Applications/app_queue
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) app_queue_v3.c.patch
Description:This patch handles the case where the penalty parameter for a queue member received via the manager in a 'QueuePenalty' action is negative. If a negative value is submitted for a member penalty, this is reject with an error.

Disclaimer on file.
Comments:By: Mark Michelson (mmichelson) 2007-12-13 19:32:44.000-0600

This is a good idea, but I think a better execution of the patch would be to put the check for a negative penalty into the set_member_penalty function. That way all methods of setting the member penalty will undergo the same check. I realize this means removing existing checks from the CLI command and dialplan function which do the same thing, but I think it's always a good idea to avoid duplication of code when possible.
By: Tomás Laureano Peralta Tormey (laureano) 2007-12-13 20:07:45.000-0600

I agree with you putnopvut. I will provide an improved patch tomorrow.
Thank you for your comments.
By: Tomás Laureano Peralta Tormey (laureano) 2007-12-17 07:34:14.000-0600

Sorry for the delay, my development box was broken during the weekend. I will provide the patch in the next days.
By: Tomás Laureano Peralta Tormey (laureano) 2007-12-18 14:59:02.000-0600

I uploaded a new version of the patch. This version:
* Checks the penalty in the function set_member_penalty, as requested by putnopvut.
* Check the syntax for the CLI command (it was broken).
* Fix a bug when logging if the incorrect parameter was the queue name or the interface.
By: Mark Michelson (mmichelson) 2007-12-18 15:18:50.000-0600

The patch looks good with one exception:

+ } else if (strcmp(a->argv[4], "on") || strcmp(a->argv[6], "in")) {

The problem is that this will have unpredictable results if the command "queue set penalty 4 on SIP/1000" were issued since a->argv[6] is out of bounds. Thanks for the patch though. We appreciate all the patches you submit!
By: Tomás Laureano Peralta Tormey (laureano) 2007-12-19 11:19:36.000-0600

I've uploaded a new version of the patch with the correct checking of the number of arguments and the arguments passed from the CLI, as requested putnopvut.
By: Digium Subversion (svnbot) 2007-12-19 17:14:30.000-0600

Repository: asterisk
Revision: 94124

U   trunk/apps/app_queue.c

------------------------------------------------------------------------
r94124 | mmichelson | 2007-12-19 17:14:29 -0600 (Wed, 19 Dec 2007) | 7 lines

1. Unify the check for a penalty < 0 into the set_member_penalty code.
2. Fix an error when checking the CLI command for setting a member's penalty.
3. Fix a logging error if the incorrect parameter was the queue name or interface.

(closes issue ASTERISK-11032, reported and patched by Laureano)


------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=94124