in reply to Passing NULL pointer through XS without "Use of uninitialized value in subroutine entry"

First, keep in mind I've never used XS. I can't guarantee the following works or even its soundness.

If you check the system typemap (lib/ExtUtils/typemap), you'll see the definition of T_PTROBJ. (What is T_OBJECT?) We can create a new rule based on T_PTROBJ to suit our needs.

# typedef struct bar* Bar; # typedef struct type_foo* Foo; TYPEMAP ####### Bar T_NULLABLE_PTROBJ Foo T_NULLABLE_PTROBJ INPUT ##### T_NULLABLE_PTROBJ if ( !SvOK( $arg ) ) { /* if not defined */ $var = NULL; } else if ( sv_derived_from( $arg, \"${ntype}\" ) ) { IV tmp = SvIV( (SV*)SvRV( $arg ) ); $var = INT2PTR( $type, tmp ); } else { Perl_croak( aTHX_ \"$var is not of type ${ntype}\" ) } OUTPUT ###### T_NULLABLE_PTROBJ if ( $var ) /* if defined */ sv_setref_pv( $arg, \"${ntype}\", (void*)$var ); else $arg = &PL_sv_undef;

The usage would be

my $foo = undef; my $bar = Foo::create_bar( $foo );

Update: I don't think addressing the problem in the typemap is the best method. This makes pointers of that type always nullable, whereas that should be decided on a per-function (and even per-argument) basis. That means we should addresses the issue in the XS stub.

# typedef struct bar* Bar; # typedef struct type_foo* Foo; # In typemap: # Bar T_PTROBJ # Foo T_PTROBJ struct bar* create_bar(foo_sv = 0) SV* foo_sv CODE: struct type_foo* foo = NULL; if ( !foo_sv ) { /* Use default of NULL. */ } else if ( !SvOK( foo_sv ) ) { /* Use default of NULL. */ } else if ( sv_derived_from( foo_sv, 'type_foo' ) ) { IV tmp = SvIV( (SV*)SvRV( foo_sv ) ); foo = INT2PTR( struct type_foo*, tmp ); } else { Perl_croak( aTHX_ "\$foo is not of type foo_type" ) } RETVAL = my_c_create_bar(foo); OUTPUT: RETVAL

Bonus: The = 0 and the if ( !foo_sv ) allows the parameter to be omitted. Feel free to remove this.

Update: Now uses tabs in typemap, as required.

Update: struct bar* and struct type_foo* are no good for type T_PTROBJ — search for T_PTROBJ_SPECIAL in perlxs — so I added typedefs.

  • Comment on Re: Passing NULL pointer through XS without "Use of uninitialized value in subroutine entry"
  • Select or Download Code

Replies are listed 'Best First'.
Re^2: Passing NULL pointer through XS without "Use of uninitialized value in subroutine entry"
by kscaldef (Pilgrim) on Aug 22, 2006 at 23:30 UTC
    Somewhat inspired by your suggestion, and taking some things from the original generated C code, I've currently got something like this:
    struct bar* create_event(foo_sv) SV* foo_sv struct type_foo* foo = NULL; CODE: if ( SvROK(foo_sv)) { SV *tmp = (SV*) SvRV(foo_sv); if ( SvOK (tmp)) { foo = (struct type_foo*) SvIV(tmp); } } else { warn( "Foo::create_bar() -- foo is not an SV reference" ); XSRETURN_UNDEF; } RETVAL = my_c_create_bar(foo); OUTPUT: RETVAL
    which successfully avoids the warning message I was getting originally. Unfortunately, this is one of those bits of code someone else wrote originally and didn't write tests for, so now I need to go and actually write some tests to make sure this works right.

      What's the problem with what I provided? You've introduced so many bugs!

      • Your function now accepts any scalar as an argument.
        create_bar(123); is accepted.
        create_bar('abc'); is accepted.
        create_bar(['a', 'b']); is accepted.
        create_bar({key => 'val'}); is accepted.

      • If the argument is a reference, it will unconditionally be treated as an object of type type_foo, even if it isn't even an object.

      • warn gets executed when you pass undef.

      • Do you even know what XSRETURN_UNDEF does? I don't, but I'm pretty sure it's not used properly.

      You should be using type T_PTROBJ if you didn't accept undef, so you should base your code on T_PTROBJ.

      Updated

        Your version doesn't work with the usage I gave initially, that's what :-)

        I agree there are bugs in this, but they are bugs that were present initially. Because this happens to be a very heavily used, very mission-critical piece of code, that unfortunately has no existing unit tests, I'm loathe to make anything more than the absolute minimal change to get rid of this warning message. For that reason what I did was to look at the generated C code and ensured that the only change was to add the particular required check against undef (the SvOK(tmp) check) to avoid the warning in this case.

        Addressing each of your points:

        • This is incorrect. The SvROK check means that only references are accepted. I've verified that 123 and 'abc' are not accepted. The two that are references are accepted, and that is indeed a bug, albeit a preexisting one.
        • Indeed, a preexisting bug
        • Yes, that is what the current interface does and is expected for that usage.
        • No, but it's what gets generated by the original XS code

        Can you explain to me exactly the difference between T_OBJECT and T_PTROBJ? I can't find anywhere in the perl docs that explains the standard typemaps, but maybe I'm not looking in the right place.