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

Re: [Perl/perl5] 7a2b6a: perltie.pod: rework example code so EXTENDis a no-op

Thread Next
From:
demerphq
Date:
January 30, 2020 09:28
Subject:
Re: [Perl/perl5] 7a2b6a: perltie.pod: rework example code so EXTENDis a no-op
Message ID:
CANgJU+XnzW3rinXPKvnbzCA1cU1ZSzetHkSUy5LjwY4GYc=XnA@mail.gmail.com
FWIW I'd love a sanity check/second set of eyes on auditing calls to
av_extend().

Cheers
Yves

On Thu, 30 Jan 2020, 16:50 Yves Orton, <noreply@github.com> wrote:

>   Branch: refs/heads/smoke-me/fix_issue_17496
>   Home:   https://github.com/Perl/perl5
>   Commit: 7a2b6adaf10fef81aca987d114e3e4a8fc52c36a
>
> https://github.com/Perl/perl5/commit/7a2b6adaf10fef81aca987d114e3e4a8fc52c36a
>   Author: Yves Orton <demerphq@gmail.com>
>   Date:   2020-01-30 (Thu, 30 Jan 2020)
>
>   Changed paths:
>     M pod/perltie.pod
>
>   Log Message:
>   -----------
>   perltie.pod: rework example code so EXTEND is a no-op
>
> Most tied array implementations can and should NO-OP the EXTEND
> method, and the sample code should not conflate EXTEND with STORESIZE.
>
> EXTEND is actually less usefull used by the core than it could be
> as AvMAX() does not have an equivalent tied method. So we cannot
> check if we need to extend.
>
> This is related to [rt.cpan.org #39196] / Issue #17496.
>
>
>   Commit: ab58e8252e124b80c9e3df8bea3eeaa2d2f01dfa
>
> https://github.com/Perl/perl5/commit/ab58e8252e124b80c9e3df8bea3eeaa2d2f01dfa
>   Author: Yves Orton <demerphq@gmail.com>
>   Date:   2020-01-30 (Thu, 30 Jan 2020)
>
>   Changed paths:
>     M av.c
>     M pad.c
>     M pp.c
>     M pp_hot.c
>     M scope.c
>
>   Log Message:
>   -----------
>   av.c, pad.c, pp.c, pp_hot.c, scope.c: resolve fencepost errors related
> to av_extend()
>
> apparently over the years there has been confusion about what the
> count argument to av_extend() is for, and at least some of our
> code has been incorrectly assuming it means the *index* in the
> array that one wants to write to, and not the number of elements
> that one expects to be available to write to. This means that
> part of the code was calling av_extend with arguments one less
> than they should have been.
>
> One such example is the code in pp_aassign() which passes in 1 less
> than it should, and the code in av_extend() for tied hashes would
> then add 1 back, thus making the tied interface seem to behave
> properly. However this is wrong, and it shows up in
>
>    @tied_array = sort @tied_array;
>
> which correctly calls av_extend with the number of elements in
> the array, which in turn would cause tied hashes to EXTEND one
> more element than they should, which for something like Tie::File
> causes the creation of an empty element at the end of the file.
> It is a little debatable whether Tie::File::EXTEND should be a
> no-op or not (I lean towards not once the internals are fixed)
> but it is certainly the case that what we were doing before this
> was not correct.
>
> This fixes [rt.cpan.org #39196] Issue #17496, although does not
> add tests, which will come in a follow up patch once I have time
> and I have seen the smoke tests from this patch.
>
>
> Compare:
> https://github.com/Perl/perl5/compare/7a2b6adaf10f%5E...ab58e8252e12
>

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