Why, given that SV *a is created in the Perl code and is not going out of scope, do I have to increment the ref count when I return it to Perl in order to avoid Attempt to free unreferenced scalar: SV 0x182445c, Perl interpreter: 0x2240d4.?

SV* test( SV *a, SV *b ) {
[download]

You make quite inconsistent thing in your C subroutine, at the very first step. You either replace SV *a with a new SV or do not, based on its constant-ness. Its a hole for scalar leaks. And may be your comment "// Why do I need to increment the ref count?" will go away. What will happen if you do not do "REFCNT_inc" exactly? Will the subroutine work for non-constant scalars?

Why not write following way:

SV* test( SV *a, SV *b ) {
    SV *c = newSVsv(a);
    sv_catsv(c,b);
    return c;
}
[download]

You make quite inconsistent thing in your C subroutine, at the very first step. You either replace SV *a with a new SV or do not, based on its constant-ness. Its a hole for scalar leaks.

I make a copy of the SV*a if it is readonly because otherwise I will get Modification of a read-only value attempted at c:\test\test.pl line 24. when I attempt to append to it. Otherwise, there is no need to copy rw scalars.

What will happen if you do not do "REFCNT_inc" exactly? Will the subroutine work for non-constant scalars?

If I do not increment the refcount, I get <c>

c:\test>test
billfred
fred
billfred
Attempt to free unreferenced scalar: SV 0x182445c, Perl interpreter: 0
+x2240d4 at c:\test\test.pl line 32.
1fred
Attempt to free unreferenced scalar: SV 0x182445c, Perl interpreter: 0
+x2240d4 at c:\test\test.pl line 35.
1
Attempt to free unreferenced scalar: SV 0x182445c, Perl interpreter: 0
+x2240d4.
Attempt to free unreferenced scalar: SV 0x182445c, Perl interpreter: 0
+x2240d4.
[download]

Why not write following way: ...

Using your version, which is close to where I started, I get the following results:

c:\test>test
billfred
Use of uninitialized value in subroutine entry at c:\test\test.pl line
+ 35.
fred
billfred
1fred
11
[download]

Note:

• The warning if SV*a was undefined.
• How the modification does not change the source.

  Ie. With my version, '1' becomes '1fred' becomes '1fred1'.

  With yours, '1' becomes '1fred', then becomes '11'.

---

Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.

Lingua non convalesco, consenesco et abolesco. -- Rule 1 has a caveat! -- Who broke the cabal?

"Science is about questioning the status quo. Questioning authority".

In the absence of evidence, opinion is indistinguishable from prejudice.

I tried and indeed got results you described.

But given "my version" behaves as it should be. Please explain why it is not.

You need concatenation SV subroutine - you get it. You receive 'undefined' warning when SV is indeed undefined, this warning could perfectly avoided, but chunk of code do what it is asked to.

#! perl -slw
use strict;

use Inline C => << '__C__', NAME => 'test', CLEAN_AFTER_BUILD => 0;
#include <stdio.h>

SV* test( SV *a, SV *b ) {
    SV *c = newSVsv(a);
    sv_catsv(c,b);
    return c;
}
__C__

print test( 'bill', 'fred' );

my( $p, $q ) = ( 'fred' );
print test( $q, $p );

$q = 'bill';
print test( $q, $p );

$q = 1;
print test( $q, $p );

$p = 1;
print test( $q, $p );
[download]

Right now, all is behaving, even w/o refcnt mess.
Or do you want inplace editing of scalar? Then you need to pass a reference to SV and in the very beginning of subroutine dereference it once!

BR,
Vadim.

Drawing on the code that has so far been presented, it looks to me that you might want:

SV* test( SV *a, SV *b ) {
    SV * c;
    if(SvREADONLY(a)) {
      c = newSVsv(a);
      sv_catsv(c, b);
      return c;
      }

    sv_catsv(a,b);
    c = newSVsv(a);
    return c;
}
[download]

I don't know how to avoid the "uninitialized value in subroutine entry" warning (apart from the obvious). Predicting whether you're going to get it or not is a little tricky. Of the 2 following subs, only test5() produces the warning for me:

SV * test4(SV * a, SV * b){return newSVuv(42);}
SV * test5(char * a, char * b){return newSVuv(42);}
[download]

And if I comment out the sv_catsv() call in the code that vkon posted, then the warning disappears. Obviously the exact nature of what the xsub does has a bearing on whether we get that warning.

Cheers,
Rob

#! perl -slw
use strict;

use Inline C => << '__C__', NAME => 'test', CLEAN_AFTER_BUILD => 0;
#include <stdio.h>

SV* test( SV *a, SV *b ) {
  SV * n = newSVsv( a );
 
    if( !SvPOK( n ) && ( SvNOK( n ) || SvIOK( n ) || SvUOK( n ) ) ) //
+ numeric
        SvPV_nolen( n );             // Force a string rep

    if( !SvPOK( n ) )      // Still nothing, must be undef?
        sv_setpv( n, "" ); // Make it the null string to stop (one pos
+sible)
        // Use of uninitialized value in subroutine entry from sv_cats
+v

    
    if(SvPOK( b ) || ( !SvPOK( b ) && ( SvNOK( b ) || SvIOK( b ) || Sv
+UOK( b ) ) ) ){
      sv_catsv( n, b );
    }
    return n;
}

__C__

print test( 'bill', 'fred' );

my( $p, $q ) = ( 'fred' );
print test( $q, $p );

$q = 'bill';
print test( $q, $p );

$q = 1;
print test( $q, $p );

$p = 1;
print test( $q, $p );
__OUTPUT__
billfred
fred
billfred
1fred
11
[download]

As for the "Attempt to free unreferenced scalar" warning, you can avoid that one of two ways.

You have to decide whether your XS algorithm is the equivalent of $a .= $b or $a = $a . $b. If there's a possibility that $a might be read-only, then your only choice is $a = $a . $b.

If you want $a .= $b, then your Inline C function should be a void function rather than returning an SV*. If you want $a = $a . $b, then you need to create a new scalar and return it.

You should not ever return the SAME scalar that was passed in. Right now, the scalar is having its reference count dropped twice -- once when you leave the XS function, and once when @_ is cleaned up after that function returns. That's why you have to increment it to avoid the freeing error. But you shouldn't do it that way -- you should either return a new scalar, or void.

Since you want the impossible (appending to read-only scalars), I can't supply a script which solves your problem, but here's one that hopefully points the way:

#!/usr/bin/perl
use strict;
use warnings;

use Inline C => <<'END_C';

SV*
concat(SV *a, SV *b) {
    SV *c;
    
    if (SvOK(a))
        c = newSVsv(a);
    else 
        c = newSVpvn("", 0);

    if (SvOK(b))
        sv_catsv(c, b);

    return c;
}

void
append(SV *a, SV *b) {
    if (!SvOK(a))
        sv_setpvn(a, "", 0);
    if (SvOK(b))
        sv_catsv(a, b);
}

END_C

my @pairs = (
    [ 'bill', undef  ],
    [ 'bill', 'fred' ],
    [ undef,  'fred' ],
    [ 1,      undef  ],
    [ 1,      1      ],      
);

test($_) for @pairs; 

sub test {
    my $pair = shift;
    my $c = concat(@$pair);
    print "$c\n"; 
    append(@$pair);
    print "$pair->[0]\n";
}
[download]

he scalar is having its reference count dropped twice -- once when you leave the XS function, and once when @_ is cleaned up after that function returns.

Thankyou. That was the bit I was not getting.

The reason for wanting to handle the case of readonly inputs is that I cannot guarentee that the caller (should this ever get into the wild), won't pass a constant that becomes the first parameter.

The reason for wanting to return the argument is the same reason that I can do

print $scalar1 .= $scalar2;
[download]

I hate modules that force me to do

my $arg = 1;
someMutator( $arg );
print $arg;
[download]

Instead of

print someMutator( 1 );
[download]

Try using Tk if its methods did not return the object:

my $widget = Tk::SomeWidget->new( ... );
$widget->Add( ... );
$widget->pack( ... );
[download]

Instead of

my $widget = Tk::SomeWidget->new( 
    ... 
)->Add(
   ...
)->pack( ... );
[download]

So, now I understand why the increment is necessary, can you clarify what is wrong (technically rather than preference), with doing an increment to counter one of the decrements?

---

Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.

Lingua non convalesco, consenesco et abolesco. -- Rule 1 has a caveat! -- Who broke the cabal?

"Science is about questioning the status quo. Questioning authority".

In the absence of evidence, opinion is indistinguishable from prejudice.

The reason for wanting to handle the case of readonly inputs is that I cannot guarentee that the caller (should this ever get into the wild), won't pass a constant that becomes the first parameter.

YAGNI - if you don't need, don't code for it. It's only causing you issues. Instead, the better solution would be to, for the nonce:

• die a horrible death if the first param is readonly
• Write a comment as to why
• Finish the coding task and move on

---

My criteria for good software:

1. Does it work?
2. Can someone else come in, make a change, and be reasonably certain no bugs were introduced?

perl/Tk is messy

Use SvOK(sv) to test for undef-ness -- then you can skip all those other tests.

Unfortunately,

    if( !SvOK( sv ) )
[download]

is not equivalent to

    if( !SvPOK( sv ) && ( SvNOK( sv ) || SvIOK( sv ) || SvUOK( sv ) ) 
+)
[download]

---

Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.

Lingua non convalesco, consenesco et abolesco. -- Rule 1 has a caveat! -- Who broke the cabal?

"Science is about questioning the status quo. Questioning authority".

In the absence of evidence, opinion is indistinguishable from prejudice.

It's not equivalent, but it does what you want it to do, doesn't it? You want to stringify the SV under all circumstances, but you don't want to see any undef warnings. SvOK tests for undef-ness. Check SvOK, and if the scalar is not defined, set its PV to an empty string. Doesn't that work?

--
Marvin Humphrey
Rectangular Research ― http://www.rectangular.com

use warnings;
use Inline C => Config =>
    CLEAN_AFTER_BUILD => 0,
    BUILD_NOISY => 1;

use Inline C => <<'EOC';

SV * foo1(SV * a) {
     SV * c = newSVsv(a);
     return c;
}

SV * foo2(SV * a) {
     return SvREFCNT_inc(a);
}

EOC

$x = 'hello';
print foo1($x);
print foo1(" world\n");
print foo2($x);
print foo2(" world\n");

__END__

D:\pscrpt\inline>perl try.pl
hello world
hello world
[download]

This still doesn't answer the question as to why foo2() receives as pre-existing scalar makes no changes to it and returns it to the caller, and suddenly we have an unreferenced scalar message.

Which scalar suddenly becomes mortal just by virtue of being passed into a subroutine and returned? As there is only one scalar involved in foo2(), it can only be SV *a, but that brings me back to the question of why?

---

Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.

Lingua non convalesco, consenesco et abolesco. -- Rule 1 has a caveat! -- Who broke the cabal?

"Science is about questioning the status quo. Questioning authority".

In the absence of evidence, opinion is indistinguishable from prejudice.

first of all, get rid of following concept:

SV* test( SV *a, SV *b ) {
    if( SvREADONLY( a ) )        // Readonly? Take a copy.
        a = newSVpv( SvPVX( a ), 0 );
[download]

What do you do here? Under some conditions you will creat new SV under other equivalent conditions it will not. What code below that should think of a - should it free it or not (in terms of refcounting - should it rely refcnt inc or not?)

Hence your uncertainity on whether refcount it.