[asterisk-dev] [Code Review] 3823: Stasis: Allow configuration of message types to disallow

[Mark Michelson]

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



trunk/main/stasis_message.c
<https://reviewboard.asterisk.org/r/3823/#comment23112>

    Can you describe the reason this function was made NULL-safe? Did you encounter a situation in testing where a NULL stasis_message_type was passed to this function?
    
    Making this function NULL-safe and returning NULL may have a ripple effect that is not addressed by this changeset. Callers of this function assume that this function will never return NULL.
    
    I suspect that code paths that would call this function with a NULL stasis_message_type are not ever going to execute since the message type was declined from being created in the first place.
    
    My thought process on this is, if it's acceptable to have a NULL stasis_message_type passed to this function, then callers of this function need to be prepared to deal with a NULL return. On the other hand, if it's never expected for a code path to ever pass a NULL stasis_message_type to this function, then the NULL check in this function should be changed to an assertion instead.


- Mark Michelson


On July 21, 2014, 5:37 p.m., opticron wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3823/
> -----------------------------------------------------------
> 
> (Updated July 21, 2014, 5:37 p.m.)
> 
> 
> Review request for Asterisk Developers and Corey Farrell.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This introduces stasis.conf and a mechanism to prevent certain message types from being published.
> 
> 
> Diffs
> -----
> 
>   trunk/tests/test_stasis_channels.c 419109 
>   trunk/tests/test_stasis.c 419109 
>   trunk/main/stasis_message.c 419109 
>   trunk/main/stasis_cache.c 419109 
>   trunk/main/stasis.c 419109 
>   trunk/include/asterisk/stasis.h 419109 
>   trunk/configs/samples/stasis.conf.sample PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/3823/diff/
> 
> 
> Testing
> -------
> 
> Tested by causing the stasis unittests to fail with the following stasis.conf configuration:
> [declined_message_types]
> decline=TestMessage
> 
> 
> Thanks,
> 
> opticron
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140721/46e8e733/attachment.html>