Front page | perl.perl5.porters |
Postings from January 2017
Re: [PATCH] Documentation patch to recommend newSVpvn over newSVpv.
Thread Previous
|
Thread Next
From:
Colin Newell
Date:
January 7, 2017 16:51
Subject:
Re: [PATCH] Documentation patch to recommend newSVpvn over newSVpv.
Message ID:
CAKPO2a4132OaXDcStY8L5L2C8QXhJd7xBN=n8L51UGWY0VJCNw@mail.gmail.com
The problem with that function is that it doesn't require all strings
to be null terminated. It's a really strange halfway house. If you
pass in null terminated strings all the time then it's safe enough.
If you don't then it works fine so long as you never pass in a zero
length string. The truth is that I think this function should
generally be avoided, if you really have nul terminated strings and
you don't want to calculate their length yourself then it is an
option, but the fact that it does a comparison to check whether to run
strlen makes me think that's not ideal. I did look for an alternative
that assumed nul terminated strings as then I could have pointed to
that as one alternative, and the newSVpvn for the rest, but I couldn't
spot one.
I've attached the patch as an attachment, unmodified for now. The
fact that it uses strlen is indeed an implementation detail, but in
the case of this function I think it's worth airing it's dirty
laundry. I want to make it clear that it's not really an ideal option
for either type of string, but I did wonder if I was clear enough
about what to do with nul terminated strings, or perhaps my concern
about the extra if before running strlen is unfounded?
Colin.
On 7 January 2017 at 16:03, Sawyer X <xsawyerx@gmail.com> wrote:
> Thank you for the patch, Colin!
>
> It is possible to attach it as a patch file? I've tried applying it with
> no success. This would also make sure your name is provided in.
>
>
> On 01/03/2017 09:09 PM, Colin Newell wrote:
>> ---
>> sv.c | 11 ++++++++++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/sv.c b/sv.c
>> index 83d82fc..bb158bf 100644
>> --- a/sv.c
>> +++ b/sv.c
>> @@ -9323,7 +9323,16 @@ SV is set to 1. If C<len> is zero, Perl will
>> compute the length using
>> C<strlen()>, (which means if you use this option, that C<s> can't have embedded
>> C<NUL> characters and has to have a terminating C<NUL> byte).
>>
>> -For efficiency, consider using C<newSVpvn> instead.
>> +This function can cause reliability issues if you are likely to pass in
>> +empty strings that are not null terminated because it will run
>> +strlen on the string and potentially run past valid memory.
>> +
>> +Using C<newSVpvn> is safer and more efficient regardless of the type
>> +of string passed in.
>> +
>> +Safer for non C<NUL> terminated strings, and more efficient for both
>> +strings as it will not have to make an additional comparison to determine
>> +whether or not to run strlen.
>
> This seems somewhat repetitive for me, as well as explicitly referring
> to strlen() which is an implementation detail.
>
> What do you think about the following?
>
> This function requires all strings to be C<NUL> terminated. For
> safety and performance, consider using C<newSVpvn> instead.
Thread Previous
|
Thread Next