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]

BR,
Vadim.

Right now, all is behaving, even w/o refcnt mess

The code in the OP does exactly what I need it to do. The equivalent of Perl's $scalar1 .= $scalar2;. Your version does not.

I want, and need to accumulate the appended data in the original SV.

The question is why do I need to increment the ref count, not how to avoid doing so. The problem with your approach is that you are completely discarding the benefits of Perl's dynamic string management.

Your code is roughly equivalent to

my $scalar3 = $scalar1;
$scalar3 .= $scalar2;
[download]

Which is not what I need. I would have to do

my $scalar3 = $scalar1;
$scalar3 .= $scalar2;
$scalar1 = $scalar3;
[download]

to get what I need and that is very wasteful as this will be called many (100s) of times to append 4 bytes each time.

---

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 want, and need to accumulate the appended data in the original SV.

That's easy, but in this case you must work not with SV but refernce to SV, and dereference it properly.

Let us start with perl stub of what you're trying to achieve

sub test {
  my $ref = shift;
  $$ref .= shift; # concatenate, but do efficiently at C level, workin
+g with preallocated space
}
[download]

If this is not what you need, then try expressing in terms of Perl sub your ideas, and we'll translate those to XS... or C.

When you enter a sub, parameters to it are mostly copied (well, sometimes they are aliased but relying on this knowledge will not help you understanding, so let us assume they are copied), so you will not gain speed unless you work with reference.

Regarding your OP code, when you make an illusion of correctly working code, but with mystery "Why do I need to increment the ref count?" this only means you got a SV leak. I can explain it, if needed.

PS sorry my frustrated reply, but once you'll explain your subroutine better I will reply better :)

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]

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

The code I posted in the OP now does what I need it to do. Basically the XS equivalent of Perl's

$scalar1 .= $scalar2;
[download]

I don't want to create a new scalar, except in the specific instance of a readonly input. The routine will be called many times and I need to accumulate the results in the scalar.

Duplicating the (already appended to), scalar in order to avoid using SvREFCOUNT_inc() is extremely wasteful and completely negates the benefits of perl's dynamically allocated string management. You have a scalar with n bytes, you call sv_catsv() to add m bytes to it. Having done that, you make a complete copy of m+n bytes in order to avoid the unreferenced scalar warning? This makes no sense to me, especially as the scalar of n bytes originates from Perl and has never gone out of scope.

My question still remains, why am I getting that warning?

The question is are there any other things that I need to test for and handle in order that I can handle any pair of SVs as input.

And if I comment out the sv_catsv() call ...

That is the (other) problem. If you call an SV that is undef as the first parameter to sv_catsv(), (which it usually will be on the first call for a particular SV), you get the warning. Hence the need for

    if( !SvPOK( a ) )      // Still nothing, must be undef?
        sv_setpv( a, "" ); // Make it the null string to stop (one pos
+sible)
        // Use of uninitialized value in subroutine entry from sv_cats
+v
[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.

You can't concatenate onto a readonly scalar, in either Perl or XS.

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

use Inline C => <<'END_C';

void 
readonly(SV *sv) {
    SvREADONLY_on(sv);
}

END_C

my $foo = "foo";
readonly($foo);

# This dies with: 
# "Modification of a read-only value attempted at cat_readonly.plx lin
+e 16."
$foo .= "bar";

# unreachable:
print "$foo\n";
[download]

So, if you have to deal with the possibility that your first scalar might be readonly, you have to do $a = $a . $b or its equivalent in XS rather than $a .= $b. It might be wasteful, but it's the only way.

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