Re: [asterisk-dev] [Code Review] 4128: func_jitterbuffer: fix frame leaks

[Corey Farrell]

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

(Updated Nov. 2, 2014, 1:35 a.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 427019


Bugs: ASTERISK-22409
https://issues.asterisk.org/jira/browse/ASTERISK-22409


Repository: Asterisk


Description
---

These changes are not controversial and fix a memory leak so they are now split 
from r3603.

In 12+ these changes apply to main/abstract_js.c instead of 
funcs/func_jitterbuffer.c.


Diffs
-

  /branches/11/main/abstract_jb.c 426593 
  /branches/11/funcs/func_jitterbuffer.c 426593 

Diff: https://reviewboard.asterisk.org/r/4128/diff/


Testing
---

tests/funcs/func_jitterbuffer against 13 no longer leaks.


Thanks,

Corey Farrell

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 4128: func_jitterbuffer: fix frame leaks

[rmudgett]

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

Ship it!


Ship It!

- rmudgett


On Oct. 31, 2014, 12:20 p.m., Corey Farrell wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4128/
> ---
> 
> (Updated Oct. 31, 2014, 12:20 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22409
> https://issues.asterisk.org/jira/browse/ASTERISK-22409
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> These changes are not controversial and fix a memory leak so they are now 
> split from r3603.
> 
> In 12+ these changes apply to main/abstract_js.c instead of 
> funcs/func_jitterbuffer.c.
> 
> 
> Diffs
> -
> 
>   /branches/11/main/abstract_jb.c 426593 
>   /branches/11/funcs/func_jitterbuffer.c 426593 
> 
> Diff: https://reviewboard.asterisk.org/r/4128/diff/
> 
> 
> Testing
> ---
> 
> tests/funcs/func_jitterbuffer against 13 no longer leaks.
> 
> 
> Thanks,
> 
> Corey Farrell
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 4128: func_jitterbuffer: fix frame leaks

[Corey Farrell]

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

(Updated Oct. 31, 2014, 1:20 p.m.)


Review request for Asterisk Developers.


Bugs: ASTERISK-22409
https://issues.asterisk.org/jira/browse/ASTERISK-22409


Repository: Asterisk


Description
---

These changes are not controversial and fix a memory leak so they are now split 
from r3603.

In 12+ these changes apply to main/abstract_js.c instead of 
funcs/func_jitterbuffer.c.


Diffs (updated)
-

  /branches/11/main/abstract_jb.c 426593 
  /branches/11/funcs/func_jitterbuffer.c 426593 

Diff: https://reviewboard.asterisk.org/r/4128/diff/


Testing
---

tests/funcs/func_jitterbuffer against 13 no longer leaks.


Thanks,

Corey Farrell

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 4128: func_jitterbuffer: fix frame leaks

[rmudgett]

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



/branches/11/funcs/func_jitterbuffer.c


Those ast_frfree's can be done now for completeness.


- rmudgett


On Oct. 30, 2014, 9:43 p.m., Corey Farrell wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4128/
> ---
> 
> (Updated Oct. 30, 2014, 9:43 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22409
> https://issues.asterisk.org/jira/browse/ASTERISK-22409
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> These changes are not controversial and fix a memory leak so they are now 
> split from r3603.
> 
> In 12+ these changes apply to main/abstract_js.c instead of 
> funcs/func_jitterbuffer.c.
> 
> 
> Diffs
> -
> 
>   /branches/11/main/abstract_jb.c 426804 
>   /branches/11/funcs/func_jitterbuffer.c 426804 
> 
> Diff: https://reviewboard.asterisk.org/r/4128/diff/
> 
> 
> Testing
> ---
> 
> tests/funcs/func_jitterbuffer against 13 no longer leaks.
> 
> 
> Thanks,
> 
> Corey Farrell
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 4128: func_jitterbuffer: fix frame leaks

[Corey Farrell]

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

(Updated Oct. 30, 2014, 10:43 p.m.)


Review request for Asterisk Developers.


Changes
---

Fix uninitialized variables in abstract_jb.c that caused additional 
ast_frfree's to crash.


Bugs: ASTERISK-22409
https://issues.asterisk.org/jira/browse/ASTERISK-22409


Repository: Asterisk


Description
---

These changes are not controversial and fix a memory leak so they are now split 
from r3603.

In 12+ these changes apply to main/abstract_js.c instead of 
funcs/func_jitterbuffer.c.


Diffs (updated)
-

  /branches/11/main/abstract_jb.c 426804 
  /branches/11/funcs/func_jitterbuffer.c 426804 

Diff: https://reviewboard.asterisk.org/r/4128/diff/


Testing
---

tests/funcs/func_jitterbuffer against 13 no longer leaks.


Thanks,

Corey Farrell

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 4128: func_jitterbuffer: fix frame leaks

[Corey Farrell]

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

(Updated Oct. 30, 2014, 9:32 p.m.)


Review request for Asterisk Developers.


Bugs: ASTERISK-22409
https://issues.asterisk.org/jira/browse/ASTERISK-22409


Repository: Asterisk


Description
---

These changes are not controversial and fix a memory leak so they are now split 
from r3603.

In 12+ these changes apply to main/abstract_js.c instead of 
funcs/func_jitterbuffer.c.


Diffs (updated)
-

  /branches/11/funcs/func_jitterbuffer.c 426804 

Diff: https://reviewboard.asterisk.org/r/4128/diff/


Testing
---

tests/funcs/func_jitterbuffer against 13 no longer leaks.


Thanks,

Corey Farrell

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 4128: func_jitterbuffer: fix frame leaks

[Corey Farrell]

> On Oct. 30, 2014, 8:32 p.m., rmudgett wrote:
> > /branches/11/funcs/func_jitterbuffer.c, lines 309-310
> > 
> >
> > ast_frfree(frame) before dup replaces frame.

This caused a segmentation fault in tests/funcs/func_jitterbuffer.


> On Oct. 30, 2014, 8:32 p.m., rmudgett wrote:
> > /branches/11/funcs/func_jitterbuffer.c, line 315
> > 
> >
> > ast_frfree(frame) before replacing with null frame.

This caused a segmentation fault in tests/funcs/func_jitterbuffer.


- Corey


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


On Oct. 30, 2014, 8:06 p.m., Corey Farrell wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4128/
> ---
> 
> (Updated Oct. 30, 2014, 8:06 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22409
> https://issues.asterisk.org/jira/browse/ASTERISK-22409
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> These changes are not controversial and fix a memory leak so they are now 
> split from r3603.
> 
> In 12+ these changes apply to main/abstract_js.c instead of 
> funcs/func_jitterbuffer.c.
> 
> 
> Diffs
> -
> 
>   /branches/11/funcs/func_jitterbuffer.c 426593 
> 
> Diff: https://reviewboard.asterisk.org/r/4128/diff/
> 
> 
> Testing
> ---
> 
> tests/funcs/func_jitterbuffer against 13 no longer leaks.
> 
> 
> Thanks,
> 
> Corey Farrell
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 4128: func_jitterbuffer: fix frame leaks

[rmudgett]

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



/branches/11/funcs/func_jitterbuffer.c


This will free the passed in frame when the old one is replaced.

if (res == AST_JB_IMPL_OK) {
+   if (jbframe != frame) {
+   ast_frfree(frame);
+   }
frame = &ast_null_frame;
+   } else if (jbframe != frame) {
+   ast_frfree(jbframe);
}




/branches/11/funcs/func_jitterbuffer.c


You don't need the if test.  Just do it.



/branches/11/funcs/func_jitterbuffer.c


While I was here I added a blank line after the variable declaration.



/branches/11/funcs/func_jitterbuffer.c


ast_frfree(frame) before dup replaces frame.



/branches/11/funcs/func_jitterbuffer.c


ast_frfree(frame) before replacing with null frame.


- rmudgett


On Oct. 30, 2014, 7:06 p.m., Corey Farrell wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4128/
> ---
> 
> (Updated Oct. 30, 2014, 7:06 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22409
> https://issues.asterisk.org/jira/browse/ASTERISK-22409
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> These changes are not controversial and fix a memory leak so they are now 
> split from r3603.
> 
> In 12+ these changes apply to main/abstract_js.c instead of 
> funcs/func_jitterbuffer.c.
> 
> 
> Diffs
> -
> 
>   /branches/11/funcs/func_jitterbuffer.c 426593 
> 
> Diff: https://reviewboard.asterisk.org/r/4128/diff/
> 
> 
> Testing
> ---
> 
> tests/funcs/func_jitterbuffer against 13 no longer leaks.
> 
> 
> Thanks,
> 
> Corey Farrell
> 
>

-- 
_
-- 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