On Monday 19 July 2010 22:08:00 Denys Vlasenko wrote:
> On Tuesday 20 July 2010 03:19, Rob Landley wrote:
> > > http://busybox.net/downloads/fixes-1.17.0/busybox-1.17.0-ask_terminal.p
> > >atch
> > >
> > > This patch will make e.g. hush to query window width using ESC [ 6 n
> > > only if ioctl(TIOCGWINSZ) fails or returns bogus value of 0.
> > >
> > > You need to not only confirm that it builds, but also try it
> > > on the serial line and let me know whether it actually
> > > does the trick. It may fail to work if ctl(TIOCGWINSZ) on the serial
> > > line lies, that is, returns some invented nonzero width and height
> > > and doesnt indicate the error.
> >
> > Ok, let's see....
> >
> > Nope, vi still thinks it's 80x25. (Which is odd because
> > CONFIG_FEATURE_VI_ASK_TERMINAL is enabled.)
> >
> > Let's see... the ioctl is returning 0 (success) but leaving win.ws_row
> > and win.ws_col at 0 if they were initialized to 0. And your code is
> > washing that through:
> >
> > static int wh_helper(int value, int def_val, const char *env_name, int
> > *err) {
> > if (value == 0) {
> > char *s = getenv(env_name);
> > if (s) {
> > value = atoi(s);
> > /* If LINES/COLUMNS are set, pretent that there
> > is * no error getting w/h, this prevents some ugly * cursor tricks by our
> > callers */
> > *err = 0;
>
> ** note that this *err = 0 thing happens _only_
> if LINES/COLUMNS are overridded in environment.
Yes, but _after_ the if(value==0) statement, we then have:
if (value <= 1 || value >= 30000)
value = def_val;
I.E. if the value passed in was 0, the default value is assigned.
> Hmmm, it should be a bug then, but I don't see where:
>
> int FAST_FUNC get_terminal_width_height(int fd, unsigned *width, unsigned
> *height) {
> struct winsize win;
> int err;
>
> win.ws_row = 0;
> win.ws_col = 0;
> err = ioctl(fd, TIOCGWINSZ, &win) != 0 || win.ws_row == 0;
> ** we get err = 1 here, since win.ws_row == 0
Ok. But these next lines don't check that:
> if (height)
> *height = wh_helper(win.ws_row, 24, "LINES", &err);
> if (width)
> *width = wh_helper(win.ws_col, 80, "COLUMNS", &err);
All they care about is that the pointers "height" and "width" are not null.
(They're not dereferencing them, they're just checking that the calling
function was querying that property. If so we call wh_helper(), which will
replace a 0 value with the default value.
So regardless of what get_terminal_width_height() is returning, *width and
*height get set to default values.
And since the response from the probe returns arbitrarily later (nothing waits
for it, not even the 1/3 second timeout vi uses after an escape key), it will
presumably continue on to use those values for some time, and reset them to
the defaults every time it probes for them.
Rob
--
GPLv3: as worthy a successor as The Phantom Meanace, as timely as Duke Nukem
Forever, and as welcome as New Coke.
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
