[Asterisk-code-review] json: Take advantage of new API's. (asterisk[16])
Richard Mudgett
asteriskteam at digium.com
Fri Sep 21 15:48:32 CDT 2018
Richard Mudgett has posted comments on this change. ( https://gerrit.asterisk.org/10194 )
Change subject: json: Take advantage of new API's.
......................................................................
Patch Set 2:
(2 comments)
https://gerrit.asterisk.org/#/c/10194/1/main/json.c
File main/json.c:
https://gerrit.asterisk.org/#/c/10194/1/main/json.c@619
PS1, Line 619: return ast_json_pack("{s: o, s: o, s: o}",
> It looks like this change causes a non-NULL return, apparently "s?" causes invalid UTF to become NUL […]
Neither of the patches to jansson fix the bug I was pointing out. The error struct is set with the NULL pointer error message regardless of if it is allowed or not. Thus there cannot be any further errors reported.
"{s:s?}"
pack_string() determines if the NULL is allowed by checking for the '?' in the format. It then calls read_string() but doesn't tell that function that a NULL string is acceptable. read_string() calls set_error() for the NULL string. Now no other errors will be set by set_error() for the pack operation.
After looking at read_string() again, it can determine itself if the NULL is optional by checking if the t variable is '?' before calling set_error().
As far as the invalid UTF-8 error on optional strings is concerned, read_string() is not setting the s->has_error = 1 flag which would cause the pack to fail and we would report the error.
https://gerrit.asterisk.org/#/c/10194/1/main/json.c@750
PS1, Line 750: "number", json_party_number(&party->number),
: "name", json_party_name(&party->name),
: "subaddress", json_party_subaddress(&party->subaddress));
> json_party_number / json_party_name / json_party_address all return NULL if !valid so I think this i […]
Yes, you are correct. Not sure how I missed that.
--
To view, visit https://gerrit.asterisk.org/10194
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-MessageType: comment
Gerrit-Change-Id: I8382d28d7d83ee0ce13334e51ae45dbc0bdaef48
Gerrit-Change-Number: 10194
Gerrit-PatchSet: 2
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Fri, 21 Sep 2018 20:48:32 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180921/64d46d29/attachment.html>
More information about the asterisk-code-review
mailing list