[asterisk-dev] [Code Review] 4186: stringfields: Fix regression from fix for unintentional memory retention caused by ast_string_fields_copy

[Matt Jordan]

> On Nov. 18, 2014, 12:45 p.m., rmudgett wrote:
> > Ship It!
> 
> wdoekes wrote:
>     Corey: is this an issue in 12+ only because the code is different? Or did the test simply not fail in 11-?
>     
>     While I hope the former, from your comment [1] on the ticket I cannot deduce which it is.
>     
>     [1] "It appears that the issue I thought applied to 1.8/11 does not exist. So this issue applies to 12+ only."
>
> 
> Corey Farrell wrote:
>     Originally I thought that __ast_string_field_ptr_grow could change the value of __p__, but Richard corrected me on this.  This is the only issue I thought I found in ast_string_field_ptr_set_by_fields.  The issue that does exist and is being fixed is with ast_string_fields_copy, which is 12+ only.  Maybe I should have said "So this ticket applies to 12+ only."

Which is in line with the unit test failures as well.


- Matt


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


On Nov. 18, 2014, 12:36 p.m., Corey Farrell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4186/
> -----------------------------------------------------------
> 
> (Updated Nov. 18, 2014, 12:36 p.m.)
> 
> 
> Review request for Asterisk Developers and rmudgett.
> 
> 
> Bugs: ASTERISK-24535
>     https://issues.asterisk.org/jira/browse/ASTERISK-24535
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> ast_string_fields_copy relies on the fact that __ast_string_field_release_active never previously zeroed pool->used, so keeping the existing pointer was "ok".  Setting each field to __ast_string_field_empty after releasing the memory seems to resolve the issue.
> 
> 
> Diffs
> -----
> 
>   /branches/12/include/asterisk/stringfields.h 428167 
> 
> Diff: https://reviewboard.asterisk.org/r/4186/diff/
> 
> 
> Testing
> -------
> 
> Full testsuite against 12.  I had 17 failures, but that is normal on my system.  I re-ran the 17 tests without this patch, they still failed.
> 
> 
> Thanks,
> 
> Corey Farrell
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20141119/be9a075d/attachment.html>