http://www.perlmonks.org?node_id=560338


in reply to Re^7: perlembed: mortalize an AV or not (misc)
in thread perlembed: mortalize an AV, get "Attempt to free unreferenced scalar" - don't mortalize, leaks memory

Okay! Last try, I hope. Does this look right?

void test(char *str) { dSP; ENTER; SAVETMPS; PUSHMARK(SP); AV *arr = (AV *)sv_2mortal((SV *) newAV()); SV *arr_value = sv_2mortal(newSVpv("test", 0)); av_push( arr, arr_value); SvREFCNT_inc(arr_value); XPUSHs(sv_2mortal(newRV_inc((SV *)arr))); HV *hash = (HV *)sv_2mortal((SV *)newHV()); SV *hash_value = sv_2mortal(newSVpv("test", 0)); SvREFCNT_inc( hash_value ); if ( NULL == hv_store( hash, "key", strlen("key"), hash_value, 0) +) { SvREFCNT_dec( hash_value ); FREETMPS; LEAVE; return; } XPUSHs(sv_2mortal(newRV_inc((SV *)hash))); PUTBACK; call_pv("TestPM::test_leak", G_SCALAR); SPAGAIN; SV *retval = POPs; PUTBACK; FREETMPS; LEAVE; }

Thanks again, tye.

--
edan

Replies are listed 'Best First'.
Re^9: perlembed: mortalize an AV or not (misc)
by demerphq (Chancellor) on Jul 11, 2006 at 09:29 UTC

    No it really doesnt look right to me. Have a look through the sources for similar code. I dont think youll see so many calls to sv_2mortal. I suggest using something like the XS that comes with DBI, or the XS that comes with List::Util for examples.

    I realize that given my earlier mistaken reply you probably arent inclined to rate my opinion highly, but I strongly encourage you to look into this further. Its sufficiently different looking from XS and core code ive seen before that it rings alarm bells in my head. Especially given that you arent using newRV_noinc() anywhere, despite perlguts specifically saying you should be.

    ---
    $world=~s/war/peace/g

Re^9: perlembed: mortalize an AV or not (misc)
by tye (Sage) on Jul 11, 2006 at 15:26 UTC

    I'd probably only SvREFCNT_inc() if the "insert" code succeeded. That also means that you don't need to SvREFCNT_dec().

    I think you'd be better off not having to remember to FREETMPS; LEAVE; if you return(), so I'd go for a simpler XS sub that lets the framework insert such stuff for you and call a regular C sub that can return() when it feels like it and the calling XS sub will do the proper clean-up.

    demerphq is surely correct that this style isn't typical. But I beleive it is correct in its handling of ref counts and also understand the motivation for the style.

    - tye