[sr-dev] memory leak in memcached module

[Daniel-Constantin Mierla]

Hello,

On 10/4/13 2:43 PM, Henning Westerholt wrote:
> Am Freitag, 4. Oktober 2013, 12:16:32 schrieb Charles Chance:
>> Sorry hit reply only, first time.
>>
>> Would you like me still to update to use mcd_free or leave it with you?
> Hi Charles,
>
> just commit the patch, looks good for me. The only small thing I noticed is
> that you changed the indention in the variable definition in pv_set_mcd_expire
> from tabs to space, would be great if you could adapt this.
>
> With regards to the other wrapper (mcd_free) that Daniel mentioned, this is
> only used as callback function for the library. I'll clarify this in the
> comments that this is the purpose of this code.
my comment was related to the fact that I haven't seen an explicit 
pkg_malloc() for the returned value from the libmacached function. I 
assumed it is going to use the callback mcd_malloc(), based on 
libmemcached api docs, which say that the result has to be released by 
caller.

 From that perspective, as the result is allocated with mcd_malloc(), it 
looks more consistent if is going to be freed with mcd_free(). A change 
in mcd_malloc() would naturally trigger an appropriate change in 
mcd_free() (e.g., using shm_malloc() instead of pkg_malloc()), while now 
if someone changes mcd_malloc() and mcd_free() would have to know that 
has to change the pkg_free() in the other part of the code.

So, in short, using pkg_free() now solves the issue, it is ok, just that 
doesn't look coherent.

Cheers,
Daniel

-- 
Daniel-Constantin Mierla - http://www.asipto.com
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
Kamailio Advanced Trainings - Berlin, Nov 25-28; Miami, Nov 18-20, 2013
   - more details about Kamailio trainings at http://www.asipto.com -