[Asterisk-code-review] AMI: Escape string values. (asterisk[13])

[Mark Michelson]

Mark Michelson has posted comments on this change.

Change subject: AMI: Escape string values.
......................................................................


Patch Set 2: Code-Review-1

(3 comments)

https://gerrit.asterisk.org/#/c/560/2/main/utils.c
File main/utils.c:

Line 1627: char ast_escape_sequences[] = {
         : 	'\a', '\b', '\f', '\n', '\r', '\t', '\v', '\\', '\'', '\"', '\?', '\0'
         : };
         : 
         : /*
         :  * Standard escape sequences output map (has to maintain matching
         :  * order with ast_escape_sequences).
         :  */
         : static char escape_sequences_map[] = {
         : 	'a', 'b', 'f', 'n', 'r', 't', 'v', '\\', '\'', '"', '?'
         : };
escape_sequences_map has no corresponding entry for '\0'.


Line 1654: 		if (strchr(to_escape, *s)) {
         : 			/*
         : 			 * See if the character to escape is part of the standard escape
         : 			 * sequences. If so we'll have to use its mapped counterpart
         : 			 * otherwise just use the given value.
         : 			 */
         : 			char *c = strchr(ast_escape_sequences, *s);
         : 			*p++ = '\\';
         : 			*p = c ? escape_sequences_map[c - ast_escape_sequences] : *s;
It seems like this may be optimizeable if to_escape is ast_escape_sequences.

In such a case, you could eliminate the second strchr() call.


Line 1686: 	if (!(res = (char*)ast_calloc(sizeof(char), size))) {
Coding guidelines say not to have the cast here.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib55a5b84fe0481b0f2caaaab68c566f392c0aac0
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-HasComments: Yes