Re: [Bug-readline] Improving support for rl_set_screen_size's restrictions

[Chet Ramey]

On 3/3/19 5:13 AM, Saagar Jha wrote:
> Hi,
> 
> GDB had a bug, which was fixed recently
> <https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=23031e319207140e76a9c18e308995fe28322b58>,
> where it would pass large values to rl_set_screen_size (which multiplies
> the arguments it’s given) and lead to undefined behavior. On the
> gdb-patches mailing list, it was mentioned
> <https://sourceware.org/ml/gdb-patches/2019-02/msg00250.html> that there is
> no documented maximum size for the rows or columns parameters. Is this
> something that Readline would be willing to improve, either through better
> documentation or better handling? I can  send in a patch for this if this
> is something that would be appreciated.

It seems like the issue is mismatched assumptions. Readline assumes the
parameters to rl_set_screensize correspond to the physical screen
dimensions. It uses these values to calculate the number of characters
that can fit on the physical screen, so it can modify redisplay when
the current line exceeds the screen dimensions.

Gdb seems to assume these dimensions correspond to some virtual panel,
that may or may not be identical to the physical screen. But then again,
it doesn't have to do any redisplay, so it's not of any consequence.

Readline initializes the physical screen dimensions itself, and uses
rl_set_screen_size as a hint from the application about what it wants,
but assumes the application will not set the screen size greater than
the physical screen dimensions.

If Gdb only uses the `unlimited' value internally, and knows whether or
not the user has requested an unlimited number of rows or columns, why
not pass -1 as the appropriate parameter to rl_set_screen_size so as to
not modify readline's internal notion of the screen size?

Chet

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
                 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRU    address@hidden    http://tiswww.cwru.edu/~chet/