Front page | perl.perl5.porters |
Postings from January 2017
Re: [PATCH] Documentation patch to recommend newSVpvn over newSVpv.
Thread Previous
From:
Colin Newell
Date:
January 7, 2017 19:07
Subject:
Re: [PATCH] Documentation patch to recommend newSVpvn over newSVpv.
Message ID:
CAKPO2a6nNhawKTc4y2+CR4mP1HNFfe19wwdWvSnJS2fqQq-V0g@mail.gmail.com
The function has 2 modes. One where you pass in the length, and the
other where you pass in 0 as the length, and it calculates it for you.
You can pass in NUL terminated strings or non NUL terminated strings,
but obviously you can't use all combinations.
NUL terminated strings, 0 Length # works fine
NUL terminated stirngs, Specify length # works fine
Non NUL terminated strings, 0 Length # doesn't work
Non NUL terminated strings, Specify length # works except when length is 0
It's that last case that's the big problem. It's use with null
terminated strings is perfectly safe, so perhaps that last paragraph
should just be dropped.
Colin.
On 7 January 2017 at 18:49, David Mertens <dcmertens.perl@gmail.com> wrote:
> My style-police mode got a bit alarmed when I saw three paragraphs each of
> which only contained a single sentence and I agree that the final paragraph
> seems redundant. I think this can be written a bit more crisply. I might
> take a stab at a revision in the next day or two if your or somebody else
> hasn't gone for it.
>
> Furthermore, I appreciate having this function around when I know I have a
> null-terminated string and don't want to explicitly compute its length with
> strlen. I also don't understand what you mean when you say that it could
> have problems with zero-length strings. Wouldn't there be an overrun problem
> with any non-null-terminated string?
>
> David
>
> On Sat, Jan 7, 2017 at 11:51 AM, Colin Newell <colin.newell@gmail.com>
> wrote:
>>
>> 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.
>
>
>
>
> --
> "Debugging is twice as hard as writing the code in the first place.
> Therefore, if you write the code as cleverly as possible, you are,
> by definition, not smart enough to debug it." -- Brian Kernighan
Thread Previous