[Asterisk-code-review] core: Config and XML tweaks needed for geolocation (asterisk[development/16/geolocation])

[Kevin Harwell]

Attention is currently required from: George Joseph.
Kevin Harwell has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/18067 )

Change subject: core: Config and XML tweaks needed for geolocation
......................................................................


Patch Set 5: Code-Review-1

(4 comments)

File include/asterisk/xml.h:

https://gerrit.asterisk.org/c/asterisk/+/18067/comment/d5e5f16c_11c96973 
PS5, Line 205: anm
Looks like this got missed s/anm/an


File main/config.c:

https://gerrit.asterisk.org/c/asterisk/+/18067/comment/0e1e92b7_f0e9d249 
PS5, Line 706: 		local_str = ast_str_create(AST_MAX_USER_FIELD);
Need to check for NULL here on local_str just in case


File tests/test_config.c:

https://gerrit.asterisk.org/c/asterisk/+/18067/comment/0a09a65f_eb23f012 
PS5, Line 1900: 	struct ast_variable *var;
I think this should be a const. See why below.


https://gerrit.asterisk.org/c/asterisk/+/18067/comment/5407d2a8_af0851fc 
PS5, Line 1936: 	var = (struct ast_variable *)ast_variable_find_variable_in_list(list, "bbb");
              : 	ast_test_validate(test, var != NULL);
              : 	ast_variable_list_replace_variable(&list, var, ast_variable_new("eee", "555", ""));
Personally I think the cast should be done on ast_variable_list_replace_variable, so it's obvious why constness is being cast away.

Alternatively you could make the "old" parameter on ast_variable_list_replace_variable itself be a const as the free is done on a non const pointer internal to the function. However, that slightly hides the fact the memory pointed to by the "old" pointer is itself freed as it points to the same memory. Not sure if that makes any more sense.



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

Gerrit-Project: asterisk
Gerrit-Branch: development/16/geolocation
Gerrit-Change-Id: I330a5f63dc0c218e0d8dfc0745948d2812141ccb
Gerrit-Change-Number: 18067
Gerrit-PatchSet: 5
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Attention: George Joseph <gjoseph at digium.com>
Gerrit-Comment-Date: Tue, 22 Feb 2022 21:38:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220222/fd980671/attachment.html>