[Asterisk-code-review] res/res_pjsip_messaging.c: Fix: unstable domain filed in To header (asterisk[18])

[Sean Bright]

Sean Bright has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/15727 )

Change subject: res/res_pjsip_messaging.c: Fix: unstable domain filed in To header
......................................................................


Patch Set 1:

(2 comments)

https://gerrit.asterisk.org/c/asterisk/+/15727/1/res/res_pjsip_messaging.c 
File res/res_pjsip_messaging.c:

https://gerrit.asterisk.org/c/asterisk/+/15727/1/res/res_pjsip_messaging.c@251 
PS1, Line 251: 		domain_start += 12; /* 12 is sizeof("x-to_domain") */
> To avoid memory allocation and releases of the string "x-to_domain" on each time of function call, the length is fixed, and we have already located it in the previously step.

sizeof("...") is evaluated at compile time, so my suggestion will add no runtime overhead.


https://gerrit.asterisk.org/c/asterisk/+/15727/1/res/res_pjsip_messaging.c@256 
PS1, Line 256: 		tmp_ptr = (char *)ast_malloc(domain_len + 1);
             : 			strncpy(tmp_ptr, domain_start, domain_len);
             : 			tmp_ptr[domain_len + 1]= '\0';
> going to rewrite these five line of codes:
> -			tmp_ptr = (char *)ast_malloc(domain_len + 1);
> -			strncpy(tmp_ptr, domain_start, domain_len);
> -			tmp_ptr[domain_len + 1]= '\0';
> -			pj_strdup2(tdata->pool, &uri->host, tmp_ptr);
> -			ast_free(tmp_ptr);
> 
> +                 if (NULL != (tmp_ptr = ast_strndup(domain_start, domain_len))) {
> +                         pj_strdup2(tdata->pool, &uri->host, tmp_ptr);
> +                         ast_free(tmp_ptr);
> +                 } /* Skip rewriting when ast_strndup failed, moving forward */

We do not style our 'if' statements that way, do this instead:

 tmp_ptr = ast_strndup(domain_start, domain_len)
 if (tmp_ptr) {
    /* do stuff with tmp_ptr */
 }



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/15727
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 18
Gerrit-Change-Id: Ib164d5274ffe73db15811d4decfc50e6eddcf80c
Gerrit-Change-Number: 15727
Gerrit-PatchSet: 1
Gerrit-Owner: Yang Chen <yang.chen at linuxe.org>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Sean Bright <sean at seanbright.com>
Gerrit-CC: Joshua Colp <jcolp at sangoma.com>
Gerrit-Comment-Date: Thu, 22 Apr 2021 17:31:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Yang Chen <yang.chen at linuxe.org>
Comment-In-Reply-To: Sean Bright <sean at seanbright.com>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210422/dc762464/attachment.html>