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

[opticron]

> On July 21, 2014, 2:25 p.m., Mark Michelson wrote:
> > trunk/main/stasis_message.c, lines 84-91
> > <https://reviewboard.asterisk.org/r/3823/diff/7/?file=64893#file64893line84>
> >
> >     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.

The call and subsequent crash occurred in the unittests when I was testing the feature on them. This function is not widely used and is generally only used where the type is being pulled from an existing message (which means it is non-NULL).  I'll drop the NULL-safety here and update the unit test.


- opticron


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


On July 21, 2014, 12: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, 12: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/bca5ca14/attachment-0001.html>