On 2004-01-01, at 20:16:27 +0000, [EMAIL PROTECTED] wrote:
> On Thu, Jan 01, 2004 at 09:10:28PM +0100, Marcus Holland-Moritz wrote:
> | On 2004-01-01, at 19:09:51 +0000, [EMAIL PROTECTED] wrote:
> |
> | > |
> | > | You can simply add a CLEANUP section (see 'perldoc perlxs' and search
> | > | for CLEANUP) to your wlgetbyid() XSUB and free the memory allocated
> | > | by g_strdup():
> | > |
> | > | CLEANUP:
> | > | free(str); /* or similar call to free the memory */
> | > |
> | > | This should take care of your memory leakage.
> | >
> | > Hmms.. maybe I've found some other problem.
> | > I have a function where a AV* array is created and returned.
> | > Who should delete/free this array?
> |
> | Code?
>
> Here it goes :)
>
> AV*
> dicgetvals(id, word)
> int id
> U32 word
> INIT:
> AV* array;
> CODE:
> if (id > MAXDICS || id < 0 || !dics[id]) {
> array = NULL;
> } else {
> int i;
> guint32 wid;
> float val;
> array = newAV();
> for (i=0;i<MAXENTRY;i++) {
> wid = dictionary_get_id(dics[id],word,i);
> val = dictionary_get_val(dics[id],word,i);
> if (val == 0.0) break;
> av_push(array, newSVuv(wid));
> av_push(array, newSVnv(val));
> }
> }
> RETVAL = array;
> OUTPUT:
> RETVAL
First, don't pass NULL as RETVAL. This looks to me like you want to
return an undefined value. Use XSRETURN_UNDEF instead.
There's a memory leak and it has to do with reference counts.
newAV() creates a new Perl array that initially has a reference
count of 1. As you cannot return an "array", but only an array
reference, xsubpp will take care of that and return an array
reference for you. Unfortunately, it uses newRV_inc() for that
purpose, which increments the reference count of the array.
Thus, the reference count of the array is 2, but there's only
one actual reference to the array. This way, the array will
never be freed => memory leak.
I'm not sure if that should be considered a bug in xsubpp or not.
I'll invesigate.
Until then, you should perhaps construct the array reference
directly:
SV*
dicgetvals(id, word)
int id
U32 word
INIT:
AV* array;
CODE:
if (id > MAXDICS || id < 0 || !dics[id]) {
XSRETURN_UNDEF;
} else {
int i;
guint32 wid;
float val;
array = newAV();
for (i=0;i<MAXENTRY;i++) {
wid = dictionary_get_id(dics[id],word,i);
val = dictionary_get_val(dics[id],word,i);
if (val == 0.0) break;
av_push(array, newSVuv(wid));
av_push(array, newSVnv(val));
}
}
RETVAL = newRV_noinc((SV*)array);
OUTPUT:
RETVAL
Note that I changed the return value from AV* to SV*.
BTW, if your intention was to return a _list_, that would be a
completely different thing.
> | Normally you shouldn't have to care and perl should take care
> | of freeing the AV. What you have to take care of is Reference
> | Counts. 'perldoc perlguts' has a section on it.
>
> I think I am managing reference counts correctly. All references (calls
> to this function) use my vars, which should be freed every time it goes
> out of scope.
Yup. I meant reference count at the XS/C level. You should
never have to deal with reference counting in normal perl
programs.
--
Line Printer paper is strongest at the perforations.
