Avoid using PL_na with newSVpv

2013-09-25 Thread 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)

Re: Avoid using PL_na with newSVpv

2013-09-25 Thread Jan Dubois 
On Wed, Sep 25, 2013 at 11:50 AM, Torsten Schoenfeld  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

Re: Avoid using PL_na with newSVpv

2013-09-25 Thread Torsten Schoenfeld 
On 25.09.2013 20:56, Jan Dubois wrote:
> On Wed, Sep 25, 2013 at 11:50 AM, Torsten Schoenfeld
>  wrote:
>> 
>> newSVpv (string, PL_na) // do not do this
> 
> Is this recomended anywhere?

Probably not.  But  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.

Re: Avoid using PL_na with newSVpv

2013-09-25 Thread Jan Dubois 
On Wed, Sep 25, 2013 at 12:04 PM, Torsten Schoenfeld  wrote:
> Probably not.  But  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