[Asterisk-code-review] res pjsip: Add ability to identify by Authorization username (asterisk[master])

[Kevin Harwell]

Kevin Harwell has posted comments on this change.

Change subject: res_pjsip:  Add ability to identify by Authorization username
......................................................................


Patch Set 1: Code-Review-1

(6 comments)

https://gerrit.asterisk.org/#/c/2368/1/CHANGES
File CHANGES:

Line 311:  
an extra space


https://gerrit.asterisk.org/#/c/2368/1/res/res_pjsip.c
File res/res_pjsip.c:

Line 255: 						endpoints and aors can be identified in multiple ways. Currently, the supported
s/endpoints/Endpoints


https://gerrit.asterisk.org/#/c/2368/1/res/res_pjsip/pjsip_configuration.c
File res/res_pjsip/pjsip_configuration.c:

Line 481: 	/*
        : 	 * If there's already something in the vector when we get here,
        : 	 * it's the default value so we need to clean it out.
        : 	 */
        : 	if (AST_VECTOR_SIZE(&endpoint->ident_method_order)) {
        : 		AST_VECTOR_RESET(&endpoint->ident_method_order, AST_VECTOR_ELEM_CLEANUP_NOOP);
        : 	}
What happens if "idents" is empty? Does this lose the default value?


https://gerrit.asterisk.org/#/c/2368/1/res/res_pjsip_endpoint_identifier_user.c
File res/res_pjsip_endpoint_identifier_user.c:

Line 127: 		ast_debug(3, "Could not identify endpoint by From username '%s'\n", username);
I think this should only be logged if the endpoint was not found. It might be a bit confusing if fails because it wasn't registered to use this particular identifier.


Line 148: 		ast_debug(3, "Could not identify endpoint by Authorization username '%s'\n", username);
Same for this. Only log if the endpoint is not found.


https://gerrit.asterisk.org/#/c/2368/1/res/res_pjsip_registrar.c
File res/res_pjsip_registrar.c:

Line 626: 	configured_aors = ast_strdupa(aors);
        : 	while ((aor_name = ast_strip(strsep(&configured_aors, ",")))) {
        : 		if (ast_strlen_zero(aor_name)) {
        : 			continue;
        : 		}
        : 
        : 		if (!strcmp(aor_name, id_domain)) {
        : 			ast_debug(3, "Matched username '%s' to aor '%s'\n", id_domain, aor_name);
        : 			return ast_strdup(aor_name);
        : 		}
        : 	}
Looks like this code is repeated a few times and can be abstracted out.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I30ba62d208e6f63439600916fcd1c08a365ed69d
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-HasComments: Yes