[Asterisk-code-review] new dialplan application XML (asterisk[master])

[Corey Farrell]

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

Change subject: new dialplan application XML
......................................................................


Patch Set 1: Code-Review-1

(5 comments)

https://gerrit.asterisk.org/#/c/4808/1/apps/app_xml.c
File apps/app_xml.c:

Line 23:  * 
Extra space


Line 29: /*** MODULEINFO
Asterisk can be compiled without libxml2 so you need to add <depend>libxml2</depend>.


Line 83: 	return -1;
Returning -1 from a dialplan app is extreme: it hangs up the channel.  Would probably be better to set a status variable SUCCESS/FAILURE?


Line 97:     xmlDocPtr doc; /* the resulting document tree */
Variables need to be declared at the top of the current code block (coding guidelines).


Line 131: 	    pbx_builtin_setvar_helper(chan, cur_node->parent->name, (char *)value);
I'm a bit concerned about using the standard variable space for this.  I'm unsure how big an issue this is, maybe depends on if cur_node->parent->name can return capitalized letters.  Maybe others will speak up if they think this is an issue or not.


A way to get around this would be using an ast_datastore_info to store information upon loading an XML doc, provide a dialplan function so information can be read from the datastore.

exten=> _XXXX,1,Set(XML()=${xml})
exten=> _XXXX,3,NoOp(${XML(msgtype)})
exten=> _XXXX,4,NoOp(${XML(tokensn)})
exten=> _XXXX,5,NoOp(${XML(langflag)})

In this case writing to the XML function would parse the document, store a variable list in a datastore, close the xml document.  Reading the XML function would return values from the previously parsed xml document.

The datastore can contain any structures you want.  For example it would be possible to keep the XML document open and use XPath to retrieve values on demand.  Not sure if XPath is wanted, just tossing out the idea.


-- 
To view, visit https://gerrit.asterisk.org/4808
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I507e729e2dfbe74092de89458a9bf903df1f0b23
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Sebastian Gutierrez <scgm11 at gmail.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-HasComments: Yes