[Asterisk-code-review] Fix crash when using framehook without codec (asterisk[master])

[Richard Mudgett]

Richard Mudgett has posted comments on this change.

Change subject: Fix crash when using framehook without codec
......................................................................


Patch Set 1: Code-Review-1

(3 comments)

https://gerrit.asterisk.org/#/c/1754/1//COMMIT_MSG
Commit Message:

Line 7: Fix crash when using framehook without codec
You are dealing with audiohooks not framehooks.  Also this statement and the following commit paragraph makes no sense.


https://gerrit.asterisk.org/#/c/1754/1/main/audiohook.c
File main/audiohook.c:

Line 825: 	} else {
This is going to free and rebuild the translation path for every frame.  A fairly CPU intensive operation.

I'd say change the original if test to:

if (ast_format_cmp(frame->subclass.format, in_translate->format) != AST_FORMAT_CMP_EQUAL) {
}


Line 826: 		if (in_translate->trans_pvt) {
        : 			ast_translator_free_path(in_translate->trans_pvt);
        : 		}
        : 		if (!(in_translate->trans_pvt = ast_translator_build_path(slin, frame->subclass.format))) {
        : 			return NULL;
        : 		}
If we cannot build the new translation path we should not be freeing the old path.  Otherwise, if the next frame matches the translate format that we freed the translation path for we will crash.

struct ast_trans_pvt *new_trans

new_trans = ast_translator_build_path()
if (!new_trans)
   return NULL
if (in_translate->trans_pvt)
   ast_translator_free_path(in_translate->trans_pvt)
in_translate->trans_pvt = new_path
ao2_replace(in_translate->format, frame->subclass.format)


-- 
To view, visit https://gerrit.asterisk.org/1754
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6ea7373fcc22e537cad373996136636201f4384
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Jonathan Rose <jrose at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes