[Asterisk-code-review] app queue: queue members can receive multiple calls (asterisk[11])

Richard Mudgett asteriskteam at digium.com
Fri Apr 22 10:48:52 CDT 2016


Richard Mudgett has posted comments on this change.

Change subject: app_queue: queue members can receive multiple calls
......................................................................


Patch Set 1:

(5 comments)

https://gerrit.asterisk.org/#/c/2679/1/apps/app_queue.c
File apps/app_queue.c:

Line 1687: #define MAX_CALL_ATTEMPT_BUCKETS 11
This is too small as people can have 500 calls active.  Pick a prime in the 300's.


Line 1693: 	return ast_str_hash(key);
This needs to be case insensitive.
ast_str_case_hash()


Line 1702: 	return strcmp(object_left->interface, right_key) ? 0 : CMP_MATCH | CMP_STOP;
> CMP_STOP is rarely ever needed and in this case it prevents the use of OBJ_
This needs to be case insensitive.  strcasecmp().


Line 1721: 	ao2_find(pending_members, m, OBJ_POINTER | OBJ_NODATA | OBJ_UNLINK);
Since there are now several places where the member needs to be removed from the pending_members container.  You should create a function to do it so only the one place will need to know how to actually remove the member from the container.


Line 2579: static void member_remove_from_queue(struct call_queue *queue, struct member *mem)
You should remove the member from the pending_members container here.  This will fix a reload that deletes the member from leaving the interface permanently reserved bug.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ice45a1c95b9f6f15d8a9fa709c5e5c84ffd29780
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 11
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes



More information about the asterisk-code-review mailing list