4 Oct
2013
4 Oct
'13
4:04 p.m.
Hello,
On 10/4/13 2:43 PM, Henning Westerholt wrote:
Am Freitag, 4. Oktober 2013, 12:16:32 schrieb Charles
Chance:
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 -
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.