develooper Front page | perl.perl5.porters | Postings from January 2017

Re: [PATCH] Documentation patch to recommend newSVpvn over newSVpv.

Thread Previous | Thread Next
From:
David Mertens
Date:
January 7, 2017 18:49
Subject:
Re: [PATCH] Documentation patch to recommend newSVpvn over newSVpv.
Message ID:
CA+4ieYV3t4XdijE6=RiWpURBGZp_q8kMsC3cGPOJnr+KRuwQsw@mail.gmail.com
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 | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About