[Asterisk-code-review] res_geolocation: eprofile, parsing, tests and more (asterisk[development/16/geolocation])

[Joshua Colp]

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

Change subject: res_geolocation: eprofile, parsing, tests and more
......................................................................


Patch Set 3: Code-Review-1

(4 comments)

File include/asterisk/res_geolocation.h:

https://gerrit.asterisk.org/c/asterisk/+/18127/comment/7b431891_73ada252 
PS3, Line 50: 		AST_STRING_FIELD(method);
Are you expecting lots of additional fields here?


File res/res_geolocation/geoloc_eprofile.c:

https://gerrit.asterisk.org/c/asterisk/+/18127/comment/a14ea0fd_cc465c86 
PS3, Line 50: 
Add a comment that this is safe because stringfields handles NULL


https://gerrit.asterisk.org/c/asterisk/+/18127/comment/fb286f86_4eb08837 
PS3, Line 101: 	eprofile->location_refinement = profile->location_refinement ? ast_variables_dup(profile->location_refinement) : NULL;
If any of this duplication fails, won't the effective profile be left in a bit of an undefined/not correct state?

I think we should not allow things to get in that state.


https://gerrit.asterisk.org/c/asterisk/+/18127/comment/30a1a7eb_aa5f87ce 
PS3, Line 122: 	if (ast_strlen_zero(uri)) {
Are we confident in this simple URI handling here?



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/18127
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: I50b66bd041b2a62ab329406f20dbaeef1fa68fc1
Gerrit-Change-Number: 18127
Gerrit-PatchSet: 3
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Attention: George Joseph <gjoseph at digium.com>
Gerrit-Comment-Date: Tue, 08 Mar 2022 12:04:02 +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/20220308/cec9f0ed/attachment-0001.html>