> On Oct. 29, 2014, 7:55 p.m., rmudgett wrote:
> > 1) In stringfields.h:ast_string_field_ptr_set_by_fields(), the __p__ and 
> > ptr pointers are the same by initialization so the test for *__p__ != *ptr 
> > is always false and will not release the old string value when 
> > __ast_string_field_alloc_space() allocates space for the new string value.  
> > I think this is the primary leak.
> > 
> > 2) In utils.c:__ast_string_field_ptr_grow(), the increase of pool->used 
> > doesn't seem right.  It should be increased to keep alignment similar to 
> > utils.c:__ast_string_field_alloc_space().
> > 
> > 3) I think a check needs to be added to 
> > utils.c:__ast_string_field_ptr_build_va() for the case when the string 
> > created by vsnprintf() is empty so the pool string can be set to the 
> > constant __ast_string_field_empty pointer.  (Like is done in 
> > stringfields.h:ast_string_field_ptr_set_by_fields())
> > 
> > 4) All of these fixes would apply to v1.8 as well.

1) I think I've fixed this.
2) To be honest I'm not sure how to fix this.
3) Fixed.
4) If this review is complete before the final 1.8 bug fix release.


- Corey


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


On Nov. 3, 2014, 8:29 a.m., Corey Farrell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4114/
> -----------------------------------------------------------
> 
> (Updated Nov. 3, 2014, 8:29 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24307
>     https://issues.asterisk.org/jira/browse/ASTERISK-24307
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Any time a stringfield is blanked it currently prevents any currently 
> allocated memory from being freed.  If a stringfield is repeatedly set to 
> blank then set to a non-blank value, it causes new pools to be continuously 
> allocated and never freed.
> 
> I'm unsure if the loop can be optimized, maybe the break can be re-added to 
> the original location on the condition that ptr == __ast_string_field_empty?
> 
> 
> Diffs
> -----
> 
>   /branches/11/main/utils.c 427111 
>   /branches/11/include/asterisk/stringfields.h 427111 
> 
> Diff: https://reviewboard.asterisk.org/r/4114/diff/
> 
> 
> Testing
> -------
> 
> Manual test using 
> https://github.com/elessard1/asterisk-lab/blob/master/examples/lab_stringfields_leak.c
>  to verify that old pools are now freed.
> 
> Full testsuite against Asterisk 13.
> 
> 
> 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

Reply via email to