[asterisk-dev] [Code Review] 2641: ARI: Add support for getting/setting channel and global variables.

[Jason Parker]

> On June 24, 2013, 10:02 p.m., Matt Jordan wrote:
> > /trunk/res/stasis/control.c, lines 126-128
> > <https://reviewboard.asterisk.org/r/2641/diff/1/?file=40088#file40088line126>
> >
> >     This should lock the channel as well

pbx_builtin_setvar_helper() is stupid.


> On June 24, 2013, 10:02 p.m., Matt Jordan wrote:
> > /trunk/res/stasis_http/resource_asterisk.c, lines 48-50
> > <https://reviewboard.asterisk.org/r/2641/diff/1/?file=40090#file40090line48>
> >
> >     Should specifying a variable that doesn't exist (and returns NULL here) cause a 404 or some other error code?

I don't think it should, no.  An empty variable should be treated the same as an unset variable.


> On June 24, 2013, 10:02 p.m., Matt Jordan wrote:
> > /trunk/res/stasis/control.c, lines 113-117
> > <https://reviewboard.asterisk.org/r/2641/diff/1/?file=40088#file40088line113>
> >
> >     A few things here.
> >     
> >     (1) ast_func_read can fail if the function module isn't loaded. If that occurs, we should return something from this function that causes ARI to return a 4xx or 5xx error.
> >     
> >     (2) Assigning workspace to value is unsafe here. The scope of workspace is limited to the block it is defined in. value has been told to point to something that, when it is dup'd, is technically out of scope. While this may "work", it isn't guaranteed to.

Fixed #2.

For #1, I don't know that the design of this stuff would allow for such things in a sane way.  Thoughts on the right way to do that?


- Jason


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2641/#review8955
-----------------------------------------------------------


On June 24, 2013, 5:48 p.m., Jason Parker wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2641/
> -----------------------------------------------------------
> 
> (Updated June 24, 2013, 5:48 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-21868
>     https://issues.asterisk.org/jira/browse/ASTERISK-21868
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Allows for reading channel variables (or functions), setting channel variables (or functions), getting global variables, setting global variables.
> 
> 
> Diffs
> -----
> 
>   /trunk/include/asterisk/stasis_app.h 392726 
>   /trunk/res/res_stasis_http_asterisk.c 392726 
>   /trunk/res/res_stasis_http_channels.c 392726 
>   /trunk/res/stasis/control.c 392726 
>   /trunk/res/stasis_http/resource_asterisk.h 392726 
>   /trunk/res/stasis_http/resource_asterisk.c 392726 
>   /trunk/res/stasis_http/resource_channels.h 392726 
>   /trunk/res/stasis_http/resource_channels.c 392726 
>   /trunk/res/stasis_json/resource_asterisk.h 392726 
>   /trunk/res/stasis_json/resource_channels.h 392726 
>   /trunk/res/stasis_json/resource_sounds.h 392726 
>   /trunk/rest-api/api-docs/asterisk.json 392726 
>   /trunk/rest-api/api-docs/channels.json 392726 
> 
> Diff: https://reviewboard.asterisk.org/r/2641/diff/
> 
> 
> Testing
> -------
> 
> All 4 things work.  Failures occur in the few places they would be expected (invalid function name, etc).
> 
> 
> Thanks,
> 
> Jason Parker
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130625/775ad092/attachment.htm>