How can I push an AV * onto the stack?

[Ben Bullock]

Is there a way to push an AV * onto the stack as an array?

Right now I have the following:

if (wantarray) {
EXTEND (SP, av_len (wantarray));
for (i = 0; i <= av_len (wantarray); i++) {
PUSHs (sv_2mortal (*(av_fetch (wantarray, i, 0))));
}
}

See:

https://github.com/benkasminbullock/Text-Fuzzy/blob/master/Fuzzy.xs#L163

Is there a better way to do it?

[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. Call SvREFCNT_inc_simple_void_NN on the returned SV * then call sv_2mortal. Also realize you are passing an alias back to the caller. "@arr = xsub();" will make a copy, but "$scalar = \xsub()" will let them control what is in your array. You can also return an array reference but then then the xsub won't be list context, just scalar context. GIMME_V will tell you want the caller wants. 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;").

[bulk88]

Ben Bullock wrote:

> On 25 March 2013 07:07, bulk88 <bul...@hotmail.com> wrote:
>
>
>> "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
> is the new code:
>
> 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.

>> 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

>> Also
>> realize you are passing an alias back to the caller. "@arr = xsub();" will
>> 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';

>> 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
> SvREFCNT_inc_simple_void_NN, I get a lot of errors like this:
>
> 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).