[Asterisk-code-review] json: Take advantage of new API's. (asterisk[16])

[Corey Farrell]

Corey Farrell has posted comments on this change. ( https://gerrit.asterisk.org/10194 )

Change subject: json: Take advantage of new API's.
......................................................................


Patch Set 1:

(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: s?, s: s?, s: o}",
> As far as Asterisk is concerned the change is ok. […]
It looks like this change causes a non-NULL return, apparently "s?" causes invalid UTF to become NULL which is then replaced by ast_json_null().

That said ast_json_string_create() does not report errors, it returns NULL.  Further jansson-2.11 has a bug where ast_json_pack("{s: o}", "key", NULL) does not report an error (it does correctly return NULL though).  This jansson bug is fixed in the bundled copy by patch 0017.  Either way it's not currently possible to get an "invalid UTF-8" error message for an optional string.

In any case I'll revert the changes to this function.


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));
> You need to take into account the valid flag for the name, number, and subaddress. […]
json_party_number / json_party_name / json_party_address all return NULL if !valid so I think this is already covered.



-- 
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: 1
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:11:24 +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/020c257b/attachment.html>