On Mon, Feb 6, 2012 at 10:35 AM, David Oswald <daosw...@gmail.com> wrote:
> That's more or less what I was thinking. I came up with this class
> and test case based on a test that was already part of the test suite;
> essentially the same bug but in a more elaborate class. The test is
> t/15basic.t in my github tests branch, or t/01basic.t in any of the
> current CPAN versions. The same bug must exist in several of
> Inline::CPP's POD examples, and might even be an issue in one of
> Inline::Struct's examples.
>
> Essentially any time a char* is passed in as a parameter, and then
> kept around as member data there's the issue of the pointer being
> invalidated by the ref-count of the original SV passed in and
> converted to a char* falling out to zero. The solution in this case
> is to copy the string within the class's constructor, and delete it in
> a destructor. It just caught me off-guard because the same sort of
> code is common in Inline::CPP's original test suite (which I'm now in
> the process of re-writing).
>
I'd have to check the typemap to be sure, but I'd guess that it just calls
SvPV (or SvPV_nolen). This is fine when you're just dealing with the string
in a function, but if you're adding it as a member to an object, you'll
need to copy it. I'm surprised that this was handled so cavalierly in the
old docs and tests.
David
> On Mon, Feb 6, 2012 at 5:46 AM, David Mertens <dcmertens.p...@gmail.com>
> wrote:
> > David,
> >
> > I believe this is a bug in your class's interaction with Perl. The fact
> that
> > the first set of tests passes is due to luck, I would guess. If you are
> > referencing a Perl scalar, you must increment its reference count, and
> > decrement the count on object destruction.
> >
> > There are also issues with guaranteeing that the string is ASCII, as a
> > general use case could very well have UTF-8. A char pointer would gladly
> > store that, of course, but the return value of the accessor will not
> > properly mark the SV with the UTF-8 flag.
> >
> > David
> >
> > On Feb 6, 2012 3:00 AM, "David Oswald" <daosw...@gmail.com> wrote:
> >>
> >> I stumbled across a strange issue in Inline::CPP. Here's a test to
> >> illustrate it:
> >>
> >> # Break object instantiation such that accessors fetch bad data.
> >>
> >> use strict;
> >> use warnings;
> >>
> >> use Test::More;
> >>
> >> use Inline 'C++' => <<END;
> >>
> >> class CStrTest {
> >> public:
> >> CStrTest( char* a );
> >> char* get_name();
> >> private:
> >> char* x;
> >> };
> >>
> >> CStrTest::CStrTest( char* a ) {
> >> x = a;
> >> }
> >>
> >> char* CStrTest::get_name() {
> >> return x;
> >> }
> >>
> >> END
> >>
> >>
> >> note( 'Subtest: Testing object instantiated by Class->new() syntax.' );
> >>
> >> subtest 'Object instantiated with Class->new() syntax.' => sub {
> >> plan tests => 4;
> >>
> >> my $obj1 = CStrTest->new( 'Honest' );
> >> my $obj2 = CStrTest->new( 'Lucky' );
> >> isa_ok( $obj1, 'CStrTest', '$obj1' );
> >> isa_ok( $obj2, 'CStrTest', '$obj2' );
> >> is( $obj1->get_name, 'Honest', "get_name on \$obj1 (Honest)" );
> >> is( $obj2->get_name, 'Lucky', "get_name on \$obj2 (Lucky)" );
> >>
> >> };
> >>
> >>
> >> TODO: {
> >>
> >> local $TODO = 'Tests on new_ok() objects fail. Still investigating
> >> why.';
> >>
> >> note(
> >> 'Subtest: Testing object instantiated by ' .
> >> 'Test::More::new_ok() syntax.'
> >> );
> >>
> >> subtest 'Object instantiated with new_ok().' => sub {
> >> plan tests => 4;
> >>
> >> my $obj1 = new_ok( 'CStrTest', [ 'Mickey' ], '$obj1' );
> >> my $obj2 = new_ok( 'CStrTest', [ 'Donald' ], '$obj2' );
> >> is( $obj1->get_name, 'Mickey', "get_name on \$obj1 (Mickey)" );
> >> is( $obj2->get_name, 'Donald', "get_name on \$obj2 (Donald)" );
> >> };
> >>
> >> }
> >>
> >> done_testing();
> >>
> >> __END__
> >>
> >> And the output.....
> >>
> >> # Subtest: Testing object instantiated by Class->new() syntax.
> >> 1..4
> >> ok 1 - $obj1 isa CStrTest
> >> ok 2 - $obj2 isa CStrTest
> >> ok 3 - get_name on $obj1 (Honest)
> >> ok 4 - get_name on $obj2 (Lucky)
> >> ok 1 - Object instantiated with Class->new() syntax.
> >> # Subtest: Testing object instantiated by Test::More::new_ok() syntax.
> >> 1..4
> >> ok 1 - $obj1 isa CStrTest
> >> ok 2 - $obj2 isa CStrTest
> >> not ok 3 - get_name on $obj1 (Mickey)
> >> # Failed test 'get_name on $obj1 (Mickey)'
> >> # at t/16charptr.t line 58.
> >> # got: 'Donald'
> >> # expected: 'Mickey'
> >> not ok 4 - get_name on $obj2 (Donald)
> >> # Failed test 'get_name on $obj2 (Donald)'
> >> # at t/16charptr.t line 59.
> >> # got: ''
> >> # expected: 'Donald'
> >> # Looks like you failed 2 tests of 4.
> >> not ok 2 - Object instantiated with new_ok(). # TODO Tests on new_ok()
> >> objects fail. Still investigating why.
> >> # Failed (TODO) test 'Object instantiated with new_ok().'
> >> # at t/16charptr.t line 60.
> >> 1..2
> >>
> >> It looks to me like what's happening is that when the object is
> >> instantiated using Test::More::use_ok(), the string passed in to the
> >> constructor must be falling out of scope and being garbage-collected
> >> so that the char* becomes invalid. But this isn't happening when the
> >> object is instantiated with Class->new() syntax.
> >>
> >> That's just a hunch. Here are some additional facts: There is no
> >> failure if we pass a basic data type instead of a pointer to a
> >> c-string. That's because when passing basic data types copies are
> >> made, whereas when passing pointers around there is no additional copy
> >> of the data pointed to.
> >>
> >> I'm trying to decide if this is a Test::More::use_ok bug, a bug in my
> >> test C++ code, a bug in Inline::CPP, a bug in the typemap for char*,
> >> or a bug in Inline::C.
> >>
> >> If anyone wishes to play with it, you can find it in my github repo:
> >> g...@github.com:daoswald/Inline-CPP.git
> >>
> >> Check out the 'cstr-test' branch. The specific test is t/16charptr.t
> >>
> >> Dave
> >>
> >> --
> >>
> >> David Oswald
> >> daosw...@gmail.com
>
>
>
> --
>
> David Oswald
> daosw...@gmail.com
>
--
"Debugging is twice as hard as writing the code in the first place.
Therefore, if you write the code as cleverly as possible, you are,
by definition, not smart enough to debug it." -- Brian Kernighan
