Bug 201167 - units(1) corrupts terminal when run via $()
Summary: units(1) corrupts terminal when run via $()
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Fernando Apesteguía
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-06-28 11:59 UTC by Julio Merino,+1 347 694 0576,New York City
Modified: 2023-02-02 09:14 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Julio Merino,+1 347 694 0576,New York City freebsd_committer 2015-06-28 11:59:35 UTC
Consider this from today's current:

-----
$ units -t ft in
12
$ echo "$(units -t ft in)"                                                                                                                                                                                      
12
  $
-----

When running 'units' from within $(), the terminal becomes unusable; note the staircas effect after the second invocation.  Carriage returns do not work any longer, nor do control characters such as Ctrl+D.  "reset" fixes the carriage return issue, but not everything: for example, backspace cannot work any longer.

I have tried this from both the "physical console" (in VMware), a remote session from OS X's Terminal.app, and from within tmux from such remote session.  (Do not try on the physical console; getty gets confused once you log out.)

I somehow doubt this is a units(1) issue and suspect the problem is coming from libedit, but I'm filing this as a units(1) issue because that's how the problem can be reproduced.

(As a related issue, "units -t" should not need to initialize libedit at all -- but it's doing it.  Removing that code path "hides" the problem.)
Comment 1 commit-hook freebsd_committer 2015-06-28 16:43:36 UTC
A commit references this bug:

Author: jmmv
Date: Sun Jun 28 16:43:08 UTC 2015
New revision: 284912
URL: https://svnweb.freebsd.org/changeset/base/284912

Log:
  Only initialize libedit when necessary

  The code path to support units conversions from the command line
  need not initialize neither libedit nor the history.  Therefore, only do
  that when in interactive mode.

  This hides the issue reported in PR bin/201167 whereby running commands
  of the form 'echo "$(units ft in)"' would corrupt the terminal.  The real
  issue causing the corruption most likely still remains somewhere.

  PR:		bin/201167
  Differential Revision:	D2935
  Reviewed by:	eadler

Changes:
  head/usr.bin/units/units.c
Comment 2 Julio Merino,+1 347 694 0576,New York City freebsd_committer 2015-06-28 16:47:04 UTC
The specific issue described above has been fixed, but the problems with libedit (or whatever it is) "corrupting" the terminal still remain.  I haven't debugged them further though, so keeping this bug open.
Comment 3 Julio Merino,+1 347 694 0576,New York City freebsd_committer 2015-06-30 03:15:51 UTC
Just to make things clear, I could reproduce this from:

* The physical console (as exposed by VMware).
* A remote SSH session from OS X's Terminal.app (TERM=xterm).
* A tmux session within the remote session above.

All with LANG=C LC_ALL=C LC_COLLATE=C.
Comment 4 Jilles Tjoelker freebsd_committer 2015-07-12 11:12:12 UTC
The corruption is because units does not shut down libedit via el_end(). The el_end() call is dead code, since the loop exits via exit(), not break.

Other issues: the (inhistory == 0) check is dead code since it has already crashed before that point; it should be moved directly after the history_init() call. Also, the history_end() call should probably be after the el_end() call, not before.
Comment 5 Fernando Apesteguía freebsd_committer 2023-02-02 09:14:41 UTC
I think this was collaterally addressed in c706c470e41177e3a4963a8b5907933db8e88e58 so we can close this one.