Beefy Boxes and Bandwidth Generously Provided by pair Networks
good chemistry is complicated,
and a little bit messy -LW

Re^8: perlembed: mortalize an AV or not (misc)

by edan (Curate)
on Jul 11, 2006 at 06:47 UTC ( #560338=note: print w/replies, xml ) Need Help??

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.


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.


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        

Log In?

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://560338]
and all is quiet...

How do I use this? | Other CB clients
Other Users?
Others having an uproarious good time at the Monastery: (9)
As of 2018-06-20 14:12 GMT
Find Nodes?
    Voting Booth?
    Should cpanminus be part of the standard Perl release?

    Results (116 votes). Check out past polls.