[asterisk-dev] [Code Review] 3519: media_formats: Add legacy format API and move chan_iax2, chan_h323, and chan_misdn over.

[Kevin Harwell]

> On May 1, 2014, 1:51 p.m., Kevin Harwell wrote:
> > /team/group/media_formats-reviewed/main/format_compatibility.c, lines 308-317
> > <https://reviewboard.asterisk.org/r/3519/diff/1/?file=58205#file58205line308>
> >
> >     It looks possible that this could return NULL.  Looking through some of the code I saw a few spots where this function was called but the NULL result was not checked for and there would be a possibility of a NULL pointer being dereffed.  So do those places need a NULL check, should this return some kind of empty format representing NULL, or is this something that really should never happen or are all the cases calling this know based on the passed in params that NULL won't be returned?
> 
> Joshua Colp wrote:
>     I've looked over the channel drivers that are using this (chan_iax2/chan_h323) and both either check the value directly or do check getting a NULL return value. Can you point to some specific cases?

Nope, you are correct.  After scanning back over the calls they are all appropriately "guarded".


- Kevin


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


On May 5, 2014, 8:33 a.m., Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3519/
> -----------------------------------------------------------
> 
> (Updated May 5, 2014, 8:33 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This change adds a legacy legacy format compatibility API which is used by chan_iax2, chan_h323, and chan_misdn to work in new media formats land.
> 
> 
> Diffs
> -----
> 
>   /team/group/media_formats-reviewed/main/format_compatibility.c PRE-CREATION 
>   /team/group/media_formats-reviewed/main/codec_builtin.c 413300 
>   /team/group/media_formats-reviewed/include/asterisk/translate.h 413300 
>   /team/group/media_formats-reviewed/include/asterisk/format_compatibility.h PRE-CREATION 
>   /team/group/media_formats-reviewed/include/asterisk/format_cache.h 413300 
>   /team/group/media_formats-reviewed/include/asterisk/codec.h 413300 
>   /team/group/media_formats-reviewed/codecs/codec_dahdi.c 413300 
>   /team/group/media_formats-reviewed/channels/iax2/provision.c 413300 
>   /team/group/media_formats-reviewed/channels/iax2/parser.c 413300 
>   /team/group/media_formats-reviewed/channels/h323/chan_h323.h 413300 
>   /team/group/media_formats-reviewed/channels/h323/ast_h323.cxx 413300 
>   /team/group/media_formats-reviewed/channels/chan_phone.c 413300 
>   /team/group/media_formats-reviewed/channels/chan_misdn.c 413300 
>   /team/group/media_formats-reviewed/channels/chan_iax2.c 413300 
>   /team/group/media_formats-reviewed/channels/chan_h323.c 413300 
>   /team/group/media_formats-reviewed/apps/app_meetme.c 413300 
> 
> Diff: https://reviewboard.asterisk.org/r/3519/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Joshua Colp
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140506/50f7ff4c/attachment.html>