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

[Richard Mudgett]

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

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


Patch Set 1: Code-Review-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.

However, this change is actually a step backwards because:

There is a bug in the libjanson s? handling code.  The pack_string()->read_string() sets an error condition anyway for a NULL string even though it is supposed to be valid.

This incorrectly set error condition then prevents subsequent error conditions from being reported.  Thus if context were NULL and exten had an invalid UTF-8 string we would not see the valid "invalid UTF-8" error message from exten but the erroneous NULL string error from context.


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.

"number", party->number.valid ? json_party_number(&party->number) : NULL,



-- 
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: 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 17:52:11 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180921/b3b16b18/attachment.html>