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

[Matt Jordan]

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



/trunk/include/asterisk/stasis_app.h
<https://reviewboard.asterisk.org/r/2641/#comment17616>

    Document that callers of this function are responsible for free'ing the returned string.



/trunk/res/stasis/control.c
<https://reviewboard.asterisk.org/r/2641/#comment17619>

    This needs to lock the channel



/trunk/res/stasis/control.c
<https://reviewboard.asterisk.org/r/2641/#comment17618>

    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.



/trunk/res/stasis/control.c
<https://reviewboard.asterisk.org/r/2641/#comment17620>

    This should lock the channel as well



/trunk/res/stasis_http/resource_asterisk.c
<https://reviewboard.asterisk.org/r/2641/#comment17621>

    Should specifying a variable that doesn't exist (and returns NULL here) cause a 404 or some other error code?



/trunk/rest-api/api-docs/channels.json
<https://reviewboard.asterisk.org/r/2641/#comment17617>

    I'd explicitly call out that this Gets a channel variable or a function value.
    
    The same finding applies to Setting a channel variable or a function value.


- Matt Jordan


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/20130624/a5ac3c8c/attachment-0001.htm>