Re: crashes when setting COLUMNS=0 or 1

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Jun 26, 10:33am, Peter Stephenson wrote:
} Subject: Re: crashes when setting COLUMNS=0 or 1
}
} On Tue, 26 Jun 2012 00:30:42 -0700
} Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
} > So, interesting side-effect:  If my patch for IPDEF5 is applied, then
} > settings of COLUMNS and LINES get propagated through to the tty driver
} > and can end up affecting the behavior of the parent shell (or anything
} > else that's using the terminal where those values were changed), even
} > after the shell where the assignments were made has exited.
} 
} That's not necessarily always a problem.  The parent is in case subject
} to such changes asynchronously via SIGWINCH.  The fact that it's getting
} the value a different way shouldn't make matters worse in general.

Yeah, but programs (including older versions of zsh) that don't expect
the tty driver to lie about the terminal size can be very confused.

(Aside:  ZLE can behave incredibly badly if the actual terminal width is
greater than what zterm_columns asserts, because of assumptions about what
happens with auto-wrap at the margins.)
 
} Where it's unhelpful is if you're setting COLUMNS not because anything
} has changed but as a temporary trick for some utility that tests it and
} truncates.  Then you wouldn't want the change to propagate back.

Indeed.

} This is going to be niggling if it means these two have different
} effects:
} 
} COLUMNS=20 ls
} (export COLUMNS=20; ls)

Actually those behave exactly the same; in both cases the new COLUMNS
persists after the command exits, because it has been enshrined in the
terminal driver.  Undoing of the "temporary" assignment in the first
example does not cause an explicit re-assignment of the old value, so
the driver is not reloaded.
 
} Howeve, I'm not sure what can reasonably be done.

Here's one possibility:   Never poke the new COLUMNS into the tty driver.
Actually we could just remove the entire #ifdef block -- it's been dead
code for years because (from >= 2) has never been true, because the only
place adjustwinsize(2) is referenced is via zlevarsetfn, which has never
been called because of the IPDEF5 typo.

On the other hand it might in the longer term (no pun intended) be
useful to figure out if there are circumstances where we do want this to
happen -- e.g., some assignments possibly should do it, others not, and
maybe we can pass a different "from" value in those cases.

Index: Src/utils.c
--- Src/utils.c 2012-05-07 20:54:49.000000000 -0700
+++ Src/utils.c 2012-06-26 07:07:15.000000000 -0700
@@ -1684,11 +1684,11 @@
 #ifdef TIOCGWINSZ
     if (interact && from >= 2 &&
        (shttyinfo.winsize.ws_row != ttyrows ||
         shttyinfo.winsize.ws_col != ttycols)) {
        /* shttyinfo.winsize is already set up correctly */
-       ioctl(SHTTY, TIOCSWINSZ, (char *)&shttyinfo.winsize);
+       /* ioctl(SHTTY, TIOCSWINSZ, (char *)&shttyinfo.winsize); */
     }
 #endif /* TIOCGWINSZ */
 
     if (zleactive && resetzle) {
 #ifdef TIOCGWINSZ


Without objection, I'll commit these two patches.