Re: Bug? in complist when filenames are longer than the screen

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Oct 4, 11:24pm, Andrey Borzenkov wrote:
}
} On Tuesday 04 October 2005 21:13, DervishD wrote:
} [...]
} 
} We seem to have two bugs here.

Yes, but they're closely related.

The *potential* for the infinite loop was introduced way back in
revision 1.6 of complist.c, more than 5 years ago.

} > interesting, it looks like a corner case: the bug only happens when
} > in the file listing is there a file whose name length is *exactly*
} > $COLUMNS. Longer or shorter files doesn't cause the bug to happen.
} 
} Yes, this was claimed to have been fixed at least twice. It has
} something to do with terminals (not) going to next line when getting
} NL in the last position.

I don't think that's actually related to the problem.  It could affect
the display being messed up, but a garbled display is not going to
crash the shell.

What appears to be happening is that a garbage structure is being read
from either mtab[] or mgtab[] (or more likely, both, but it is random
which one contains crash-inducing garbage).  Those are both flat arrays
representing a mapping, for every character position on the display,
of which completion (mtab) and group (mgtab) correspond to the text at
that position.  I say flat because they're not two-dimensional by lines
and columns, rather array index 0 is the top left and index $(($LINES *
$COLUMNS - 1)) is the lower right.

Or, at least, that's my interpretation of what mtab and mgtab are.
There are no comments anywhere, and Sven liked to use extremely short
variable names for the indexes into these arrays.  Note that there are
a lot of NULL elements in these arrays (for screen positions occupied
by spaces or descriptive text).

I think the problem is an interaction among the changes in rev 1.6,
rev 1.12, and rev 1.17, all of which alter the way those tables are
traversed or maintained.  The most likely one is 1.12, which attempts
to prevent the highlighted match from wrapping both horizontally and
vertically at the same time.  The trouble is -- I think -- that when
there is only one column, every time the end of the list is reached
it *appears* to be wrapping both horizontally and vertically (the
destination column never differs from the origin column, as it were).

The commitlog for 1.17 claims it only affects menu scrolling, but I'm
pretty confident from some gdb sessions that the code in the line 1035
hunk [*] and in the line 1678 hunk [*] is being executed in ordinary
menu selection as well.

[*] Line numbers from cvsweb diff display browsing CVS on SourceForge.
These no longer correspond to lines in the current source.

All of this is complicated by the fact that Sven relied on pointers
always referring to even-numbered-byte boundaries, and so bitwise-ORs
the lowest bit with 1 to differentiate "marked" from "unmarked" screen
positions.  This is hopelessly nonportable; some architectures will
crash outright as soon as the bit-flip makes the pointer invalid.
In any case, though, the inifinite loop is caused when scanning the
entire mtab array fails to find any "marked" position at all, and I
suspect (but can't yet be sure) that rev 1.12 is the reason that no
position has been marked when the selection has moved off the end of
the list and ought to wrap back to the start.

Rev 1.17 gets involved because (again: I think, not yet sure) it doesn't
properly distinguish the edge case of whether a line that is exactly the
width of the screen should be considered to "span multiple lines".

If someone else has some spare minutes and eyeballs, please ...