in reply to XS/Inline::C concat *any* two SVs.

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

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]
--
Marvin Humphrey
Rectangular Research ― http://www.rectangular.com

Replies are listed 'Best First'.
Re^2: XS/Inline::C concat *any* two SVs.
by BrowserUk (Patriarch) on May 28, 2006 at 15:39 UTC
    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.
[reply]
[d/l]
[select]
      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?
[reply]

        The problem of dealing with readonly inputs is a wide-spread one with XS code. Win32::API demonstrates this. In order to ensure against traps when passing parameters from Perl that are destined for system APIs written in C that will expect to be able to use strlen to determine string lengths, the author chooses to append a null to string inputs. In most cases this is unneccessary as Perl already does this, but given the unknowable nature of the APIs called through that interface, and the parameters they can take, it's good insurance.

        The downside is that even when passing string constants that are never modified, it forces the user to do 

        my $stringConstant = 'the constant';
$api->Call( $stringConstant, ... );

        [download]

        In order that the module can append the insurance null. This is a necessary precaution, but a PITA for the user. I wish to avoid that scenario. Creamygoodness has explained the cause of the problem I was encountering, so my post has equiped me with the information I need to deal with this situation correctly.

        The problem is a generic one with all XS code, and as I am not under time pressure to provide the solution, it is my nature to try and understand the nitty gritty of the problems I encounter so that I will be equiped to deal with them in the future. I'm a strong proponent of YAGNI when designing code, but in this case, the problem is generic enough that I will definitely need it at some point in the future, even if I decided that it isn't critical to the project that raised it.

        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.
[reply]
[d/l]
[select]
      perl/Tk is messy
[reply]
Re^2: XS/Inline::C concat *any* two SVs.
by BrowserUk (Patriarch) on May 30, 2006 at 06:38 UTC
    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.
[reply]
[d/l]
[select]

      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
[reply]

        Unfortunately not. If I substitute SvOK( sv ) for my more complex test, then I get this output from my testcases: 

        c:\test>test
billfred
Use of uninitialized value in subroutine entry at c:\test\test.pl line
+ 30.
fred
billfred
fred
fred1

        [download]

        Compare that with the output from the original 

        c:\test>test
billfred
fred
billfred
1fred
1fred1

        [download]

        The problem is that it detects that (N|I|U)OK is true, but it doesn't cause the PV to be set to reflect their contents. So, when you call sv_catsv the value in the NV|IV|UV gets discarded. Or at least that's my best interpretation of what I am seeing.

        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.
[reply]
[d/l]
[select]

In Section Seekers of Perl Wisdom