in reply to Tie::File - sorting array adds empty lines

I actually spent a few hours looking into it out of curiosity. It's a very unique bug. Like ikegami said, it only occurs in the optimized case of a tied array where the sort is of this format: @a = sort @a.

From what I can tell of gdb stumblings through Perl_pp_sort(), in the particular event of a tied array in the above format, the code branch at line 1716 of pp_sort.c will be followed (code below):

if (av && !sorting_av) { /* simulate pp aassign of tied AV */ ... av_extend(av, max); ... }

When the av_extend is called, max has the correct value that was returned from FETCHSIZE. However, the code that deals with tied arrays at the top of av_extend ends up pushing max+1 onto the stack prior to the EXTEND call:

Perl_av_extend(pTHX_AV *av, I32 key) { MAGIC * const mg = SvTIED_mg((SV*)av, PERL_MAGIC_tied); if (msg) { ... PUSHs(SvTIED_obj((SV*)av, mg)); PUSHs(sv_2mortal(newSViv(key+1))); PUTBACK; call_method("EXTEND", G_SCALAR|G_DISCARD);

I haven't the foggiest why this is, as I'm no Perl internals expert. But the result appears to be an off-by-one in the module's implementation of the EXTEND.

I think the reason it doesn't affect many other modules is that the bulk of modules that use tied arrays (that I've tested at least) have EXTEND as a no-op function ({}). Tie::File, on the other hand, actually uses the EXTEND to determine the number of records in the file. Because of this, you always end up with an extra empty record (which in this case is a newline, since that is the default record separator) because of the off-by-one.

Replies are listed 'Best First'.
Re^2: Tie::File - sorting array adds empty lines
by ikegami (Patriarch) on Sep 11, 2008 at 23:58 UTC

    Thanks. Submitting bug report for Tie::File. (Upd: CPAN RT bug #39196 )

    But the result appears to be an off-by-one in the module's implementation of the EXTEND.

    It's not an off-by-one error, at least not on the module's behalf.
    EXTEND is used to expand the internal buffer.
    STORESIZE is used to actually change the visible size of the array.
    Tie::File incorrectly treats EXTEND as STORESIZE.

      I concur. There has been a long standing bug that the tied EXTEND method was being called with 1 more than it should have been called by the perl internals sub av_extend(). This was complemented by another bug where in pp_aassign() av_extend was being called with 1 less than it should have been, cancelling each other out in a practical sense for most use cases. Since most Tie modules implement EXTEND as a NO-OP this was not noticed. Once the two fencepost errors were removed this problem in Tie::File went away. I have pushed a fix which is currently being smoke tested, assuming that fix was correct I will merge it to blead, and we might see the fix included in Perl 5.32. I would like to apologize on behalf of the perl5porters community for not getting to the bottom of this earlier.

      See https://github.com/Perl/perl5/issues/17496 for details.

      ---
      $world=~s/war/peace/g