XS: SvUPGRADE and test for lvalue

[Wolfram Humann]

This XS code can be found in many CPAN modules (e.g. Zlib.xs):

if (!SvUPGRADE(buf, SVt_PV))
croak("cannot use buf argument as lvalue");

It is used before sending "SV * buf" to a C function that will fill buf
with some string. I wanted to use the same in my code but just could not
get it to "carp" even if I passed numeric or string constants to buf.
When I say

if (SvREADONLY(buf))
croak("cannot use buf argument as lvalue");

instead (and "SvUPGRADE" separately), the test works as expected. I
looked at sv_upgrade in sv.c and found two "return TRUE;" statements but
no way for the function to return FALSE.

Did sv_upgrade change at some point, so that the test used above does
not work anymore? Any insights?

Wolfram

 

[Anno Siegel]

This XS code can be found in many CPAN modules (e.g. Zlib.xs):

if (!SvUPGRADE(buf, SVt_PV))
croak("cannot use buf argument as lvalue");

It is used before sending "SV * buf" to a C function that will fill buf
with some string. I wanted to use the same in my code but just could not
get it to "carp" even if I passed numeric or string constants to buf.
When I say

if (SvREADONLY(buf))
croak("cannot use buf argument as lvalue");

instead (and "SvUPGRADE" separately), the test works as expected. I
looked at sv_upgrade in sv.c and found two "return TRUE;" statements but
no way for the function to return FALSE.

You are right. sv_upgrade() croaks itself under some circumstances,
but when it returns, it returns true.

Did sv_upgrade change at some point, so that the test used above does
not work anymore? Any insights?

That is probably what happened. It must have returned false at some
time in the past. These days, if( !SvUPGRADE(...) ) must be considered
cargo cult.

Anno

 

[Tassilo v. Parseval]

Also sprach Wolfram Humann:

This XS code can be found in many CPAN modules (e.g. Zlib.xs):

if (!SvUPGRADE(buf, SVt_PV))
croak("cannot use buf argument as lvalue");

It is used before sending "SV * buf" to a C function that will fill buf
with some string. I wanted to use the same in my code but just could not
get it to "carp" even if I passed numeric or string constants to buf.

The piece of code you quoted is nonsense in two ways. First of all,
SvUPGRADE (although being a macro) is documented to have a return type
of 'void', according to perlapi.pod.

Secondly, SvUPGRADE() has nothing to do with the readonlyness of a
variable. It merely turns one SV into a more complex SV, not touching
the readonly-flag.

When I say

if (SvREADONLY(buf))
croak("cannot use buf argument as lvalue");

instead (and "SvUPGRADE" separately), the test works as expected. I
looked at sv_upgrade in sv.c and found two "return TRUE;" statements but
no way for the function to return FALSE.

Did sv_upgrade change at some point, so that the test used above does
not work anymore? Any insights?

Not that I am aware of. Note that sv_upgrade will actually croak when it
was unable to upgrade an SV.

Tassilo

 

[Anno Siegel]

Also sprach Wolfram Humann:


The piece of code you quoted is nonsense in two ways. First of all,
SvUPGRADE (although being a macro) is documented to have a return type
of 'void', according to perlapi.pod.

Secondly, SvUPGRADE() has nothing to do with the readonlyness of a
variable. It merely turns one SV into a more complex SV, not touching
the readonly-flag.

[olivaorts/perl/perl-5.8.6] anno% grep SvUPGRADE *.c | grep 'if '
toke.c: if (!SvUPGRADE(datasv, SVt_PVIO))

Squashed in bleadperl, AFAIK.

Anno

 

[Tassilo v. Parseval]

Also sprach Anno Siegel:

[olivaorts/perl/perl-5.8.6] anno% grep SvUPGRADE *.c | grep 'if '
toke.c: if (!SvUPGRADE(datasv, SVt_PVIO))

You know, the spot with the highest density of cargo-cult Perl and
non-Perl is actually the perl source code itself.

Squashed in bleadperl, AFAIK.

Oh, indeed. I also did a grep similar to yours before my post (on a
bleadperl synced maybed two weeks ago) and your hit was still in. Now I
re-synced and this little rotter has vanished.

Tassilo