How can I push an AV * onto the stack?

bulk88

2013-03-25 02:24:24 UTC

Post by bulk88
"PUSHs (sv_2mortal (*(av_fetch (wantarray, i, 0))));" will cause a double
free. av_fetch doesn't give you a refcount notch to own.

Thanks for spotting that. I decided to use av_undef to delete it. Here
if (wantarray) {
SV * e;
EXTEND (SP, av_len (wantarray));
for (i = 0; i <= av_len (wantarray); i++) {
e = * av_fetch (wantarray, i, 0);
SvREFCNT_inc_simple_void_NN (e);
PUSHs (sv_2mortal (e));
}
av_undef (wantarray);
}

You still leak the AV *. av_undef in perl is "@arr = ();", av_undef is
not "{my @arr; @arr =();} 0; #destroyed". av_undef does not change
refcount so the av is leaked.

Post by bulk88
Call
SvREFCNT_inc_simple_void_NN on the returned SV * then call sv_2mortal.

Thanks for this advice, which I followed, although the function
doesn't seem to be documented anywhere.

http://perldoc.perl.org/perlapi.html#SvREFCNT_inc_simple_void_NN

Post by bulk88
Also
make a copy, but "$scalar = \xsub()" will let them control what is in your
array.

I'm not sure what you mean here.

If you wrote sub getKey in XS, and ++ refcount on the SV in the hash,
then mortaled the SV, then pushed it on Perl stack, then returned from
the xsub, if the caller did a \ on the return value of the xsub, they
could control the inside of your object. |sv_mortalcopy exists for a
reason. But sv_mortalcopy is slower than ++ the refcount and mortaling
the original SV so evaluate the risk. I am not saying your code needs to
change or has this problem, just pointing out something that isn't obvious.|

sub new{return bless({key => 'name}, 'someclass');}
sub getKey {return $_[0]->{key};}
$obj = someclass->new();
my $svref = \($obj->getKey());
$$svref = 'I changed the key in the object';

Post by bulk88
In the github code, you also leaked AV * wantarray. You didn't mortal
it, or SvREFCNT_dec it or put it in a RV that itself is mortaled or
SvREFCNT_dec'd. You could also, this is high end stuff, do a memcpy from
AvARRAY to Perl stack, then do "SP += count;" (or is that "SP += count;").

I used av_undef. It seems to work. If I comment out the line about
Attempt to free unreferenced scalar: SV 0x2856ad80 at t/return-array.t line 46.
Attempt to free unreferenced scalar: SV 0x2856ad10 at t/return-array.t line 46.
Attempt to free unreferenced scalar: SV 0x2856ad40 at t/return-array.t line 46.
Attempt to free unreferenced scalar: SV 0x2856ae00 at t/return-array.t line 46.
Attempt to free unreferenced scalar: SV 0x28579cd0 at t/return-array.t line 46.
So it looks like the memory is freed correctly when the line is restored.

I assume you are talking about.

_______________________________________________________________

e = * av_fetch (wantarray, i, 0); // e is probably 1, owned by wantarray
SvREFCNT_inc_simple_void_NN (e); //e now 2, 1 owned by wantarray, other
is temporarily leaked
PUSHs (sv_2mortal (e)); //e now 2, 1 owned by wantarray, other will be
freeded by caller if the caller doesn't want it
_______________________________________________________________ if you
comment out SvREFCNT_inc_simple_void_NN, when the mortal cleanup
happens, since the av_undef happened already, refcount goes from 0 to -1
and triggers "Attempt to free unreferenced scalar"

When you newAV(), the AV has refcount 1. Just do a sv_2mortal on it, and
it will be freeded when your XSUB returns to its caller. That will drop
the refcount to 0, and then the AV is freeded during the return to
caller (in pp_entersub or pp_nextstate specifically, dont remember which).