Avoid using PL_na with newSVpv

[Torsten Schoenfeld]

I recently tracked down a difficult bug that occured only when one of my
XS modules was used in conjunction with certain other XS modules (e.g.,
XML::Parser or String::Approx). It turned out to be due to the
incorrect assumption that PL_na will always be zero. Specifically, we used

newSVpv (string, PL_na) // do not do this

when we didn't have the string's length. This breaks if some other XS
module writes to PL_na (via SvPV, most likely). So always use an
explicit zero instead:

newSVpv (string, 0)

[Jan Dubois]

On Wed, Sep 25, 2013 at 11:50 AM, Torsten Schoenfeld <kaffe...@gmx.de> wrote:
>
> newSVpv (string, PL_na) // do not do this

Is this recomended anywhere? PL_na is a legacy variable that used to
be used for

str = SvPV(sv, PL_na);

in case you don't care about the length of the string. Since this will
always write the length of the string to PL_na, I can't think of any
reason why you would assume that PL_na would always be zero.

A more efficient way is to use

str = SvPV_nolen(sv);

As more and more code gets rid of using PL_na, it becomes more and
more likely that PL_na will indeed be 0 (because it has never been
used before), but as you point out, that is a dangerous assumption.

Cheers,
-Jan

[Torsten Schoenfeld]

On 25.09.2013 20:56, Jan Dubois wrote:
> On Wed, Sep 25, 2013 at 11:50 AM, Torsten Schoenfeld
> <kaffe...@gmx.de> wrote:
>>
>> newSVpv (string, PL_na) // do not do this
>
> Is this recomended anywhere?

Probably not. But <http://grep.cpan.me/?q=newSVpv.*PL_na> does show
quite a few hits, only roughly half of which are mine. Since the bugs
this causes are hard to diagnose, I thought a public service
announcement is appropriate.

> Since this will always write the length of the string to PL_na, I
> can't think of any reason why you would assume that PL_na would
> always be zero.

I think I simply assumed that "na" stood for "not available". So I took
PL_na as a readability-improving alias for zero.

[Jan Dubois]

On Wed, Sep 25, 2013 at 12:04 PM, Torsten Schoenfeld <kaffe...@gmx.de> wrote:
> Probably not. But <http://grep.cpan.me/?q=newSVpv.*PL_na> does show
> quite a few hits, only roughly half of which are mine. Since the bugs
> this causes are hard to diagnose, I thought a public service
> announcement is appropriate.

Hmm, maybe you should email the maintainers of those modules directly
(or file bugs), if you have the time? perl-xs is kind of an obscure
list, so it is unlikely they will actually see this thread.

> I think I simply assumed that "na" stood for "not available". So I took
> PL_na as a readability-improving alias for zero.

I think in this case it means "not applicable", meaning "I don't care
what the value is".

Cheers,
-Jan