Re: How can I push an AV * onto the stack?

Ben Bullock wrote:
> On 25 March 2013 07:07, bulk88 <[email protected]> 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).