[Asterisk-code-review] app_sf: Add full tech-agnostic SF support (asterisk[master])

Kevin Harwell asteriskteam at digium.com
Wed Dec 15 18:21:10 CST 2021


Attention is currently required from: N A.
Kevin Harwell has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/17652 )

Change subject: app_sf: Add full tech-agnostic SF support
......................................................................


Patch Set 2: Code-Review-1

(3 comments)

File apps/app_sf.c:

https://gerrit.asterisk.org/c/asterisk/+/17652/comment/76275d69_434d1bf7 
PS2, Line 223: 								if (!(dsp = ast_dsp_new())) {
             : 									ast_log(LOG_WARNING, "Unable to allocate DSP!\n");
             : 									pbx_builtin_setvar_helper(chan, "RECEIVESFSTATUS", "ERROR");
             : 									ast_frfree(frame);
             : 									return -1;
             : 								}
             : 								ast_dsp_set_features(dsp, DSP_FEATURE_FREQ_DETECT);
             : 								ast_dsp_set_freqmode(dsp, freq, SF_MIN_DETECT, 16, 0);
Did you mean to dupe the dsp new, set features, and frequmode code from above here? 'dsp' is set above. If you need to reset it then you need to free it first or what you have here will cause a memory leak.


https://gerrit.asterisk.org/c/asterisk/+/17652/comment/0cecfb1b_6e926539 
PS2, Line 278: 				}
'frame' is leaked here on each loop.


File main/app.c:

https://gerrit.asterisk.org/c/asterisk/+/17652/comment/b7694072_b7e60112 
PS2, Line 870: 	freq = ast_malloc(32);
Why 32? Either make this a description #define or add a comment about the size.

Also, since this is a small, set allocation you could either declare 'freq' as stack allocated fixed array, or use ast_alloca here.

If you do change this to a stack allocated variable then you can also remove the associated ast_free below.



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/17652
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I7ec50752e9a661af639425e5d1e339f17411bcad
Gerrit-Change-Number: 17652
Gerrit-PatchSet: 2
Gerrit-Owner: N A <mail at interlinked.x10host.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Michael Bradeen <mbradeen at sangoma.com>
Gerrit-Attention: N A <mail at interlinked.x10host.com>
Gerrit-Comment-Date: Thu, 16 Dec 2021 00:21:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20211215/1d8163c7/attachment-0001.html>


More information about the asterisk-code-review mailing list