Skip Menu |

This queue is for tickets about the Class-XSAccessor-Array CPAN distribution.

Report information
The Basics
Id: 38573
Status: resolved
Priority: 0/
Queue: Class-XSAccessor-Array

People
Owner: Nobody in particular
Requestors: AGRUNDMA [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Critical
Broken in: 0.03
Fixed in: (no value)



Subject: Scalars not copied when setting
Hi Steffen, I've run into a strange bug when using this module that doesn't happen with a pure-Perl accessor module. I think the problem is that the XS code doesn't make a copy of scalars, and stores the original variable. See the attached test script. Output: foo: SV = PV(0x801d08) at 0x8017e8 REFCNT = 1 FLAGS = (PADBUSY,PADMY,POK,pPOK) PV = 0x334690 "foo"\0 CUR = 3 LEN = 4 pp->[0]: SV = PV(0x801d5c) at 0x801770 REFCNT = 1 FLAGS = (POK,pPOK) PV = 0x320d90 "foo"\0 CUR = 3 LEN = 4 xs->[0]: SV = PV(0x801d08) at 0x8017e8 REFCNT = 2 FLAGS = (PADBUSY,PADMY,POK,pPOK) PV = 0x334690 "foo"\0 CUR = 3 LEN = 4
Subject: cxsa-bug.pl
#!/usr/bin/perl use strict; use Devel::Peek; my $pp = PP->new; my $xs = XS->new; my $foo = 'foo'; warn "foo:\n"; Dump $foo; $pp->acc($foo); $xs->acc($foo); warn "pp->[0]:\n"; Dump $pp->[0]; warn "xs->[0]:\n"; Dump $xs->[0]; package PP; sub new { my $class = shift; return bless [], $class; } sub acc { return $_[0]->[0] if @_ == 1; return $_[0]->[0] = $_[1] if @_ == 2; } 1; package XS; sub new { my $class = shift; return bless [], $class; } use Class::XSAccessor::Array accessors => { acc => 0, }; 1;
What do you think of this patch? It seems to fix the issue and includes a test case.
Only in Class-XSAccessor-Array-0.03-new: Array.new.xs diff -ur Class-XSAccessor-Array-0.03/Array.xs Class-XSAccessor-Array-0.03-new/Array.xs --- Class-XSAccessor-Array-0.03/Array.xs 2008-06-22 04:45:26.000000000 -0400 +++ Class-XSAccessor-Array-0.03-new/Array.xs 2008-08-19 15:34:04.000000000 -0400 @@ -36,8 +36,7 @@ * We uses it to identify the currently running alias of the accessor. Gollum! */ const I32 index = AutoXS_arrayindices[ix]; PPCODE: - SvREFCNT_inc(newvalue); - if (NULL == av_store((AV*)SvRV(self), index, newvalue)) { + if (NULL == av_store((AV*)SvRV(self), index, newSVsv(newvalue))) { croak("Failed to write new value to array."); } XPUSHs(newvalue); @@ -56,8 +55,7 @@ PPCODE: if (items > 1) { SV* newvalue = ST(1); - SvREFCNT_inc(newvalue); - if (NULL == av_store((AV*)SvRV(self), index, newvalue)) + if (NULL == av_store((AV*)SvRV(self), index, newSVsv(newvalue))) croak("Failed to write new value to array."); XPUSHs(newvalue); } diff -ur Class-XSAccessor-Array-0.03/t/01basic.t Class-XSAccessor-Array-0.03-new/t/01basic.t --- Class-XSAccessor-Array-0.03/t/01basic.t 2008-05-03 12:59:18.000000000 -0400 +++ Class-XSAccessor-Array-0.03-new/t/01basic.t 2008-08-19 15:35:13.000000000 -0400 @@ -1,7 +1,7 @@ use strict; use warnings; -use Test::More tests => 14; +use Test::More tests => 15; BEGIN { use_ok('Class::XSAccessor::Array') }; package Foo; @@ -53,3 +53,8 @@ ok($foo->get_foo() eq '1'); ok($foo->get_bar() eq '2'); +# Make sure scalars are copied and not stored by reference (RT 38573) +my $x = 1; +$foo->set_foo($x); +$x++; +is( $foo->get_foo(), 1, 'scalar copied properly' );
Thanks for spotting and fixing the problem. I applied your patch and also fixed the corresponding bug in the Class::XSAccessor module (for hash-based objects). Sorry for the delay in replying. I was at YAPC and then on vacation. Best regards, Steffen