Skip to content

Commit ab58e82

Browse files
committed
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.
1 parent 7a2b6ad commit ab58e82

File tree

5 files changed

+6
-6
lines changed

5 files changed

+6
-6
lines changed

av.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ Perl_av_extend(pTHX_ AV *av, SSize_t key)
7272
mg = SvTIED_mg((const SV *)av, PERL_MAGIC_tied);
7373
if (mg) {
7474
SV *arg1 = sv_newmortal();
75-
sv_setiv(arg1, (IV)(key + 1));
75+
sv_setiv(arg1, (IV)(key));
7676
Perl_magic_methcall(aTHX_ MUTABLE_SV(av), mg, SV_CONST(EXTEND), G_DISCARD, 1,
7777
arg1);
7878
return;

pad.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2493,7 +2493,7 @@ Perl_padlist_dup(pTHX_ PADLIST *srcpad, CLONE_PARAMS *param)
24932493

24942494
pad1 = newAV();
24952495

2496-
av_extend(pad1, ix);
2496+
av_extend(pad1, ix+1);
24972497
PadlistARRAY(dstpad)[1] = pad1;
24982498
pad1a = AvARRAY(pad1);
24992499

pp.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4895,7 +4895,7 @@ PP(pp_aslice)
48954895
max = elem;
48964896
}
48974897
if (max > AvMAX(av))
4898-
av_extend(av, max);
4898+
av_extend(av, max+1);
48994899
}
49004900

49014901
while (++MARK <= SP) {

pp_hot.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2416,7 +2416,7 @@ PP(pp_aassign)
24162416

24172417
if (SvMAGICAL(ary) || SvREADONLY(ary) || !AvREAL(ary)) {
24182418
/* for arrays we can't cheat with, use the official API */
2419-
av_extend(ary, nelems - 1);
2419+
av_extend(ary, nelems);
24202420
for (i = 0; i < nelems; i++) {
24212421
SV **svp = &(PL_tmps_stack[tmps_base + i]);
24222422
SV *rsv = *svp;
@@ -4926,7 +4926,7 @@ Perl_clear_defarray(pTHX_ AV* av, bool abandon)
49264926
}
49274927
else {
49284928
AV *newav = newAV();
4929-
av_extend(newav, fill);
4929+
av_extend(newav, fill+1);
49304930
AvREIFY_only(newav);
49314931
PAD_SVl(0) = MUTABLE_SV(newav);
49324932
SvREFCNT_dec_NN(av);

scope.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ Perl_new_stackinfo(pTHX_ I32 stitems, I32 cxitems)
7676
Newx(si, 1, PERL_SI);
7777
si->si_stack = newAV();
7878
AvREAL_off(si->si_stack);
79-
av_extend(si->si_stack, stitems > 0 ? stitems-1 : 0);
79+
av_extend(si->si_stack, stitems > 0 ? stitems : 0);
8080
AvALLOC(si->si_stack)[0] = &PL_sv_undef;
8181
AvFILLp(si->si_stack) = 0;
8282
si->si_prev = 0;

0 commit comments

Comments
 (0)