From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12265 invoked by alias); 26 Jun 2012 14:26:40 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 30533 Received: (qmail 27759 invoked from network); 26 Jun 2012 14:26:36 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 Received-SPF: none (ns1.primenet.com.au: domain at closedmail.com does not designate permitted sender hosts) From: Bart Schaefer Message-id: <120626072614.ZM19995@torch.brasslantern.com> Date: Tue, 26 Jun 2012 07:26:14 -0700 In-reply-to: <20120626103314.61c6567b@pwslap01u.europe.root.pri> Comments: In reply to Peter Stephenson "Re: crashes when setting COLUMNS=0 or 1" (Jun 26, 10:33am) References: <20120626033802.GA3066@primenet.com.au> <120626000311.ZM19019@torch.brasslantern.com> <120626003042.ZM19270@torch.brasslantern.com> <20120626103314.61c6567b@pwslap01u.europe.root.pri> X-Mailer: OpenZMail Classic (0.9.2 24April2005) To: zsh workers Subject: Re: crashes when setting COLUMNS=0 or 1 MIME-version: 1.0 Content-type: text/plain; charset=us-ascii 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 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.