[Asterisk-code-review] res pjsip registrar.c: Update remove existing AOR contact ha... (asterisk[13])

[George Joseph]

George Joseph has posted comments on this change. ( https://gerrit.asterisk.org/6630 )

Change subject: res_pjsip_registrar.c: Update remove_existing AOR contact handling.
......................................................................


Patch Set 2: Code-Review-1

(1 comment)

https://gerrit.asterisk.org/#/c/6630/2/res/res_pjsip_registrar.c
File res/res_pjsip_registrar.c:

https://gerrit.asterisk.org/#/c/6630/2/res/res_pjsip_registrar.c@361
PS2, Line 361: static unsigned int heap_calc_height(unsigned int num_elements)
             : {
             : 	unsigned int height = 0;
             : 
             : 	while (num_elements) {
             : 		num_elements >>= 1;
             : 		++height;
             : 	}
             : 	return height;
             : }
             : 
             : static int heap_min_contact_expire_cmp(void *left, void *right)
             : {
             : 	struct ast_sip_contact *left_contact = left;
             : 	struct ast_sip_contact *right_contact = right;
             : 
             : 	/* Swap left and right to get minimum expiration time remaining heap. */
             : 	return ast_tvcmp(right_contact->expiration_time, left_contact->expiration_time);
             : }
             : 
             : static int heap_push_contact(void *obj, void *arg, int flags)
             : {
             : 	struct ast_sip_contact *contact = obj;
             : 	struct ast_heap *contact_heap = arg;
             : 
             : 	ast_heap_push(contact_heap, contact);
             : 
             : 	return 0;
             : }
             : 
             : /*!
             :  * \internal
             :  * \brief Remove excess existing contacts that expire the soonest.
             :  * \since 13.18.0
             :  *
             :  * \param contacts Container of unmodified contacts that could remove.
             :  * \param to_remove Maximum number of contacts to remove.
             :  *
             :  * \return Nothing
             :  */
             : static void remove_excess_contacts(struct ao2_container *contacts, unsigned int to_remove)
             : {
             : 	struct ast_heap *contact_heap;
             : 
             : 	ast_assert(0 < to_remove);
             : 
             : 	/* Create a minimum contact expiration time remaining heap */
             : 	contact_heap = ast_heap_create(heap_calc_height(ao2_container_count(contacts)),
             : 		heap_min_contact_expire_cmp, -1);
             : 	if (!contact_heap) {
             : 		return;
             : 	}
             : 	ao2_callback(contacts, OBJ_NODATA | OBJ_MULTIPLE, heap_push_contact, contact_heap);
             : 
             : 	/* Remove the excess contacts that expire the soonest */
             : 	while (to_remove--) {
             : 		struct ast_sip_contact *contact;
             : 
             : 		contact = ast_heap_pop(contact_heap);
             : 		if (!contact) {
             : 			break;
             : 		}
             : 
             : 		ast_sip_location_delete_contact(contact);
             : 		ast_verb(3, "Removed contact '%s' from AOR '%s' due to remove_existing\n",
             : 			contact->uri, contact->aor);
             : 		ast_test_suite_event_notify("AOR_CONTACT_REMOVED",
             : 			"Contact: %s\r\n"
             : 			"AOR: %s\r\n"
             : 			"UserAgent: %s",
             : 			contact->uri,
             : 			contact->aor,
             : 			contact->user_agent);
             : 	}
             : 
             : 	ast_heap_destroy(contact_heap);
             : }
Couldn't you have done this with a more common and less expensive vector rather than a rarely used and more expensive heap?



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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: comment
Gerrit-Change-Id: I64c107a10b70db1697d17136051ae6bf22b5314b
Gerrit-Change-Number: 6630
Gerrit-PatchSet: 2
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Comment-Date: Fri, 06 Oct 2017 13:26:36 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171006/4595f394/attachment.html>