> On May 1, 2014, 6:51 p.m., Kevin Harwell wrote: > > /team/group/media_formats-reviewed/channels/chan_h323.c, lines 1049-1050 > > <https://reviewboard.asterisk.org/r/3519/diff/1/?file=58191#file58191line1049> > > > > If no longer needed then delete, or did you mean to come back to this?
BUGBUGs added. > On May 1, 2014, 6:51 p.m., Kevin Harwell wrote: > > /team/group/media_formats-reviewed/channels/chan_h323.c, lines 2615-2618 > > <https://reviewboard.asterisk.org/r/3519/diff/1/?file=58191#file58191line2615> > > > > More of the same commented code. Ditto. > On May 1, 2014, 6:51 p.m., Kevin Harwell wrote: > > /team/group/media_formats-reviewed/channels/chan_iax2.c, lines 6282-6284 > > <https://reviewboard.asterisk.org/r/3519/diff/1/?file=58192#file58192line6282> > > > > May not be a problem, but what happens if/when these formats get > > unref'ed since it points to a global object? If the incoming frame is on > > the stack the format won't be unref'ed so should be fine, otherwise these > > might need a ref bump. It is indeed on the stack and is safe. > On May 1, 2014, 6: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? 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? - Joshua ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3519/#review11797 ----------------------------------------------------------- On April 30, 2014, 10:54 p.m., Joshua Colp wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3519/ > ----------------------------------------------------------- > > (Updated April 30, 2014, 10:54 p.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 413144 > /team/group/media_formats-reviewed/include/asterisk/translate.h 413144 > /team/group/media_formats-reviewed/include/asterisk/format_compatibility.h > PRE-CREATION > /team/group/media_formats-reviewed/include/asterisk/format_cache.h 413144 > /team/group/media_formats-reviewed/include/asterisk/codec.h 413144 > /team/group/media_formats-reviewed/codecs/codec_dahdi.c 413144 > /team/group/media_formats-reviewed/channels/iax2/provision.c 413144 > /team/group/media_formats-reviewed/channels/iax2/parser.c 413144 > /team/group/media_formats-reviewed/channels/h323/chan_h323.h 413144 > /team/group/media_formats-reviewed/channels/h323/ast_h323.cxx 413144 > /team/group/media_formats-reviewed/channels/chan_phone.c 413144 > /team/group/media_formats-reviewed/channels/chan_misdn.c 413144 > /team/group/media_formats-reviewed/channels/chan_iax2.c 413144 > /team/group/media_formats-reviewed/channels/chan_h323.c 413144 > /team/group/media_formats-reviewed/apps/app_meetme.c 413144 > > Diff: https://reviewboard.asterisk.org/r/3519/diff/ > > > Testing > ------- > > > Thanks, > > Joshua Colp > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
