[sr-dev] Acc: cdr extra2strar allocation issue and proposed fix

[Daniel-Constantin Mierla]

Hello,

On 28/07/14 18:35, Lucian Balaceanu wrote:
> Hi Daniel,
>
> Thank you for your reply.  I did not fully understand exactly where 
> this free error occurs in our code,

I didn't applied the patch to see all the cases when the 'goto error' 
can occur. Maybe it was safe with previous patch as well.

  My concern was that working with a global array, via some error 
situation, the free array happens and the array still has the valuee of 
the pointers from the previous execution and another free on such value 
will cause a crash.
> but I agree that setting pointer to NULL after free is a good 
> defensive programming practice ( and would take care of such a faulty 
> case as you described). I attach a patch where the freeing is done as 
> per your indications.

You can commit it.

Thanks,
Daniel
>
> Thank you,
> Lucian Balaceanu
>
>
>
> On 07/28/2014 04:30 PM, Daniel-Constantin Mierla wrote:
>> Hello,
>>
>> I think the cleanup of the array with allocated values is not safe 
>> enough, if there is an error.
>>
>> I thought of this situation:
>> - acc went fine for several iterations and the allocated values are freed
>> - on current iteration it fails populating properly the values and 
>> the execution jumps to free the array. .s field can be a pointer from 
>> previous iteration
>>
>> I think that .s has to be made NULL after it is freed and checked to 
>> not be null before freeing it.
>>
>> Let me know if I overlooked something there.
>>
>> On 23/07/14 08:27, Lucian Balaceanu wrote:
>>> Hello,
>>>
>>> At 1&1 we have spotted an issue related to the cdr_extra parameters: 
>>> for more than 10 string cdr_extra parameters, the addresses used by 
>>> the new parameters overwrite the previous ones (this did not happen 
>>> in 3.1, but is reproducible since at least 3.3).
>>>
>>> I attached a patch that implements a solution where we allocate 
>>> memory for the cdr extra params with pkg_malloc() and free it once 
>>> it is no longer needed.
>>> Daniel, if there is no comment related to this solution, I will 
>>> commit the patch.
>>>
>>> Thank you,
>>> Lucian Balaceanu
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> sr-dev mailing list
>>> sr-dev at lists.sip-router.org
>>> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
>>
>> -- 
>> Daniel-Constantin Mierla -http://www.asipto.com
>> http://twitter.com/#!/miconda  -http://www.linkedin.com/in/miconda
>

-- 
Daniel-Constantin Mierla - http://www.asipto.com
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sip-router.org/pipermail/sr-dev/attachments/20140729/5e67f37e/attachment.html>