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
Hello,
this is probably related to the value of pv buffer slots:
http://www.kamailio.org/wiki/cookbooks/devel/core#pv_buffer_slots
Having a way to allocate dynamically could be a good alternative. I will check the patch and come back with the remarks.
Cheers, Daniel
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@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
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@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
Hi Daniel,
Thank you for your reply. I did not fully understand exactly where this free error occurs in our code, 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.
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@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
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@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 -
Lucian Balaceanu