Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

less does not quit on q and does not react to any keys #108

Closed
al20878 opened this issue Dec 22, 2020 · 14 comments
Closed

less does not quit on q and does not react to any keys #108

al20878 opened this issue Dec 22, 2020 · 14 comments

Comments

@al20878
Copy link

al20878 commented Dec 22, 2020

I have a real VT330 terminal connected via a serial line (@9600baud rate) to a Raspberry Pi running the latest OS with all updates current.

Using less version 487, first page of the files gets rendered properly (that is, the screen cleared and filled up properly, last line shows status in reverse video), but then less stops reacting to any of the regular keys, like q, space, or even control-c: everything gets echoed literally on screen, though, and when less is killed from outside, those typed characters get processed by the shell... So in case of a few q's typed (with Enter in between then), bash prints "q: command not found"...

Here goes the relevant part of strace, showing that less reads from the terminal but does not get any input characters...

The command was: less /etc/terminfo/README

22:27:19.531615 ioctl(1, TCGETS, {c_iflags=0x2506, c_oflags=0x5, c_cflags=0x4bd, c_lflags=0xa3b, c_line=0, c_cc="\x03\x1c\x7f\x15\x04\x00\x01\x00\x11\x13\x1a\x00\x12\x0f\x17\x16\x00\x00\x00"}) = 0
22:27:19.531765 ioctl(1, TCGETS, {c_iflags=0x2506, c_oflags=0x5, c_cflags=0x4bd, c_lflags=0xa3b, c_line=0, c_cc="\x03\x1c\x7f\x15\x04\x00\x01\x00\x11\x13\x1a\x00\x12\x0f\x17\x16\x00\x00\x00"}) = 0
22:27:19.531983 brk(NULL)               = 0x3ac000
22:27:19.532092 brk(0x3cd000)           = 0x3cd000
22:27:19.532387 stat64("/home/pi/.terminfo", 0x3ac2b0) = -1 ENOENT (No such file or directory)
22:27:19.532549 stat64("/etc/terminfo", {st_dev=makedev(0xb3, 0x2), st_ino=130948, st_mode=S_IFDIR|0755, st_nlink=2, st_uid=0, st_gid=0, st_blksize=4096, st_blocks=8, st_size=4096, st_atime=1562719233 /* 2019-07-09T20:40:33.591003368-0400 */, st_atime_nsec=591003368, st_mtime=1576207324 /* 2019-12-12T22:22:04.150523445-0500 */, st_mtime_nsec=150523445, st_ctime=1576207324 /* 2019-12-12T22:22:04.150523445-0500 */, st_ctime_nsec=150523445}) = 0
22:27:19.532744 stat64("/lib/terminfo", {st_dev=makedev(0xb3, 0x2), st_ino=132301, st_mode=S_IFDIR|0755, st_nlink=15, st_uid=0, st_gid=0, st_blksize=4096, st_blocks=8, st_size=4096, st_atime=1562719236 /* 2019-07-09T20:40:36.639058177-0400 */, st_atime_nsec=639058177, st_mtime=1549905440 /* 2019-02-11T12:17:20-0500 */, st_mtime_nsec=0, st_ctime=1562719236 /* 2019-07-09T20:40:36.639058177-0400 */, st_ctime_nsec=639058177}) = 0
22:27:19.532983 stat64("/usr/share/terminfo", {st_dev=makedev(0xb3, 0x2), st_ino=165075, st_mode=S_IFDIR|0755, st_nlink=44, st_uid=0, st_gid=0, st_blksize=4096, st_blocks=8, st_size=4096, st_atime=1562719302 /* 2019-07-09T20:41:42.152236215-0400 */, st_atime_nsec=152236215, st_mtime=1562717311 /* 2019-07-09T20:08:31.728481008-0400 */, st_mtime_nsec=728481008, st_ctime=1562719302 /* 2019-07-09T20:41:42.152236215-0400 */, st_ctime_nsec=152236215}) = 0
22:27:19.533218 access("/etc/terminfo/v/vt330", R_OK) = -1 ENOENT (No such file or directory)
22:27:19.533368 access("/lib/terminfo/v/vt330", R_OK) = -1 ENOENT (No such file or directory)
22:27:19.533492 access("/usr/share/terminfo/v/vt330", R_OK) = 0
22:27:19.533633 openat(AT_FDCWD, "/usr/share/terminfo/v/vt330", O_RDONLY|O_LARGEFILE) = 3
22:27:19.533778 fstat64(3, {st_dev=makedev(0xb3, 0x2), st_ino=164402, st_mode=S_IFREG|0644, st_nlink=1, st_uid=0, st_gid=0, st_blksize=4096, st_blocks=8, st_size=1055, st_atime=1576207376 /* 2019-12-12T22:22:56-0500 */, st_atime_nsec=0, st_mtime=1572718579 /* 2019-11-02T14:16:19-0400 */, st_mtime_nsec=0, st_ctime=1576207408 /* 2019-12-12T22:23:28.779356120-0500 */, st_ctime_nsec=779356120}) = 0
22:27:19.533971 read(3, "\32\1N\0\25\0\7\0\231\0o\2vt340|dec-vt340|vt33"..., 4096) = 1055
22:27:19.534093 read(3, "", 4096)       = 0
22:27:19.534256 close(3)                = 0
22:27:19.534391 ioctl(1, TCGETS, {c_iflags=0x2506, c_oflags=0x5, c_cflags=0x4bd, c_lflags=0xa3b, c_line=0, c_cc="\x03\x1c\x7f\x15\x04\x00\x01\x00\x11\x13\x1a\x00\x12\x0f\x17\x16\x00\x00\x00"}) = 0
22:27:19.534517 ioctl(1, TCGETS, {c_iflags=0x2506, c_oflags=0x5, c_cflags=0x4bd, c_lflags=0xa3b, c_line=0, c_cc="\x03\x1c\x7f\x15\x04\x00\x01\x00\x11\x13\x1a\x00\x12\x0f\x17\x16\x00\x00\x00"}) = 0
22:27:19.534627 ioctl(1, TCGETS, {c_iflags=0x2506, c_oflags=0x5, c_cflags=0x4bd, c_lflags=0xa3b, c_line=0, c_cc="\x03\x1c\x7f\x15\x04\x00\x01\x00\x11\x13\x1a\x00\x12\x0f\x17\x16\x00\x00\x00"}) = 0
22:27:19.534743 ioctl(1, TCGETS, {c_iflags=0x2506, c_oflags=0x5, c_cflags=0x4bd, c_lflags=0xa3b, c_line=0, c_cc="\x03\x1c\x7f\x15\x04\x00\x01\x00\x11\x13\x1a\x00\x12\x0f\x17\x16\x00\x00\x00"}) = 0
22:27:19.534872 ioctl(1, TIOCGWINSZ, {ws_row=24, ws_col=80, ws_xpixel=0, ws_ypixel=0}) = 0
22:27:19.535056 ioctl(2, TIOCGWINSZ, {ws_row=24, ws_col=80, ws_xpixel=0, ws_ypixel=0}) = 0
22:27:19.535275 openat(AT_FDCWD, "/usr/bin/.sysless", O_RDONLY|O_LARGEFILE) = -1 ENOENT (No such file or directory)
22:27:19.535433 openat(AT_FDCWD, "/etc/sysless", O_RDONLY|O_LARGEFILE) = -1 ENOENT (No such file or directory)
22:27:19.535562 openat(AT_FDCWD, "/home/pi/.less", O_RDONLY|O_LARGEFILE) = -1 ENOENT (No such file or directory)
22:27:19.535727 openat(AT_FDCWD, "/home/pi/.lesshst", O_RDONLY|O_LARGEFILE) = 3
22:27:19.535856 fstat64(3, {st_dev=makedev(0xb3, 0x2), st_ino=859, st_mode=S_IFREG|0600, st_nlink=1, st_uid=1000, st_gid=1000, st_blksize=4096, st_blocks=8, st_size=163, st_atime=1608600901 /* 2020-12-21T20:35:01.877517846-0500 */, st_atime_nsec=877517846, st_mtime=1608600901 /* 2020-12-21T20:35:01.877517846-0500 */, st_mtime_nsec=877517846, st_ctime=1608600901 /* 2020-12-21T20:35:01.877517846-0500 */, st_ctime_nsec=877517846}) = 0
22:27:19.536056 read(3, ".less-history-file:\n.search\n\"und"..., 4096) = 163
22:27:19.536220 read(3, "", 4096)       = 0
22:27:19.536329 close(3)                = 0
22:27:19.536470 openat(AT_FDCWD, "/dev/tty", O_RDONLY|O_LARGEFILE) = 3
22:27:19.536638 ioctl(3, TCGETS, {c_iflags=0x500, c_oflags=0x1805, c_cflags=0xbf, c_lflags=0x8a01, c_line=0, c_cc[VMIN]=1, c_cc[VTIME]=0, c_cc="\x03\x1c\x7f\x15\x04\x00\x01\x00\x11\x13\x1a\x00\x12\x0f\x17\x00\x00\x00\x00"}) = 0
22:27:19.536760 fsync(3)                = -1 EINVAL (Invalid argument)
22:27:19.536921 ioctl(3, TCGETS, {c_iflags=0x500, c_oflags=0x1805, c_cflags=0xbf, c_lflags=0x8a01, c_line=0, c_cc[VMIN]=1, c_cc[VTIME]=0, c_cc="\x03\x1c\x7f\x15\x04\x00\x01\x00\x11\x13\x1a\x00\x12\x0f\x17\x00\x00\x00\x00"}) = 0
22:27:19.537038 ioctl(3, SNDCTL_TMR_STOP or TCSETSW, {c_iflags=0x500, c_oflags=0x1805, c_cflags=0xbf, c_lflags=0x8a01, c_line=0, c_cc[VMIN]=1, c_cc[VTIME]=0, c_cc="\x03\x1c\x7f\x15\x04\x00\x01\x00\x11\x13\x1a\x00\x12\x0f\x17\x00\x00\x00\x00"}) = 0
22:27:19.537175 ioctl(3, TCGETS, {c_iflags=0x500, c_oflags=0x1805, c_cflags=0xbf, c_lflags=0x8a01, c_line=0, c_cc[VMIN]=1, c_cc[VTIME]=0, c_cc="\x03\x1c\x7f\x15\x04\x00\x01\x00\x11\x13\x1a\x00\x12\x0f\x17\x00\x00\x00\x00"}) = 0
22:27:19.537295 rt_sigaction(SIGINT, {sa_handler=0x26464, sa_mask=[INT], sa_flags=SA_RESTORER|SA_RESTART, sa_restorer=0x76d55120}, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0
22:27:19.537441 rt_sigaction(SIGTSTP, {sa_handler=0x26420, sa_mask=[TSTP], sa_flags=SA_RESTORER|SA_RESTART, sa_restorer=0x76d55120}, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0
22:27:19.537564 rt_sigaction(SIGWINCH, {sa_handler=0x263dc, sa_mask=[WINCH], sa_flags=SA_RESTORER|SA_RESTART, sa_restorer=0x76d55120}, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0
22:27:19.537691 rt_sigaction(SIGQUIT, {sa_handler=SIG_IGN, sa_mask=[QUIT], sa_flags=SA_RESTORER|SA_RESTART, sa_restorer=0x76d55120}, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0
22:27:19.537823 stat64("/etc/terminfo/README", {st_dev=makedev(0xb3, 0x2), st_ino=132880, st_mode=S_IFREG|0644, st_nlink=1, st_uid=0, st_gid=0, st_blksize=4096, st_blocks=8, st_size=212, st_atime=1562719233 /* 2019-07-09T20:40:33.587003297-0400 */, st_atime_nsec=587003297, st_mtime=1549905440 /* 2019-02-11T12:17:20-0500 */, st_mtime_nsec=0, st_ctime=1562719233 /* 2019-07-09T20:40:33.587003297-0400 */, st_ctime_nsec=587003297}) = 0
22:27:19.538003 stat64("/etc/terminfo/README", {st_dev=makedev(0xb3, 0x2), st_ino=132880, st_mode=S_IFREG|0644, st_nlink=1, st_uid=0, st_gid=0, st_blksize=4096, st_blocks=8, st_size=212, st_atime=1562719233 /* 2019-07-09T20:40:33.587003297-0400 */, st_atime_nsec=587003297, st_mtime=1549905440 /* 2019-02-11T12:17:20-0500 */, st_mtime_nsec=0, st_ctime=1562719233 /* 2019-07-09T20:40:33.587003297-0400 */, st_ctime_nsec=587003297}) = 0
22:27:19.538200 openat(AT_FDCWD, "/etc/terminfo/README", O_RDONLY|O_LARGEFILE) = 4
22:27:19.538321 _llseek(4, 1, [1], SEEK_SET) = 0
22:27:19.538442 _llseek(4, 0, [0], SEEK_SET) = 0
22:27:19.538551 read(4, "This directory is for system-loc"..., 256) = 212
22:27:19.538722 _llseek(4, 1, [1], SEEK_SET) = 0
22:27:19.538833 fstat64(4, {st_dev=makedev(0xb3, 0x2), st_ino=132880, st_mode=S_IFREG|0644, st_nlink=1, st_uid=0, st_gid=0, st_blksize=4096, st_blocks=8, st_size=212, st_atime=1562719233 /* 2019-07-09T20:40:33.587003297-0400 */, st_atime_nsec=587003297, st_mtime=1549905440 /* 2019-02-11T12:17:20-0500 */, st_mtime_nsec=0, st_ctime=1562719233 /* 2019-07-09T20:40:33.587003297-0400 */, st_ctime_nsec=587003297}) = 0
22:27:19.539008 _llseek(4, 0, [0], SEEK_SET) = 0
22:27:19.539131 stat64("/etc/terminfo/README", {st_dev=makedev(0xb3, 0x2), st_ino=132880, st_mode=S_IFREG|0644, st_nlink=1, st_uid=0, st_gid=0, st_blksize=4096, st_blocks=8, st_size=212, st_atime=1562719233 /* 2019-07-09T20:40:33.587003297-0400 */, st_atime_nsec=587003297, st_mtime=1549905440 /* 2019-02-11T12:17:20-0500 */, st_mtime_nsec=0, st_ctime=1562719233 /* 2019-07-09T20:40:33.587003297-0400 */, st_ctime_nsec=587003297}) = 0
22:27:19.539355 write(1, "\33[?1h\33=\r", 8) = 8
22:27:19.539525 read(4, "This directory is for system-loc"..., 8192) = 212
22:27:19.539759 write(1, "This directory is for system-loc"..., 250) = 250
22:27:19.539931 read(3, 0x7e9eab7b, 1)  = ? ERESTARTSYS (To be restarted if SA_RESTART is set)
22:27:31.151247 --- SIGTERM {si_signo=SIGTERM, si_code=SI_USER, si_pid=22537, si_uid=1000} ---
22:27:31.152084 +++ killed by SIGTERM +++

Not sure why it reads from descriptor 3, though, as the terminal input is usually at 0.

more works perfectly fine, with spacebar advancing the pages, and q quitting the program...

@al20878
Copy link
Author

al20878 commented Dec 22, 2020

Full write data for descriptor 1:

22:44:12.306096 write(1, "\33[?1h\33=\r", 8) = 8
 | 00000  1b 5b 3f 31 68 1b 3d 0d                           .[?1h.=.         |
22:44:12.306299 read(4, "This directory is for system-loc"..., 8192) = 212
22:44:12.306544 write(1, "This directory is for system-loc"..., 250) = 250
 | 00000  54 68 69 73 20 64 69 72  65 63 74 6f 72 79 20 69  This directory i |
 | 00010  73 20 66 6f 72 20 73 79  73 74 65 6d 2d 6c 6f 63  s for system-loc |
 | 00020  61 6c 20 74 65 72 6d 69  6e 66 6f 20 64 65 73 63  al terminfo desc |
 | 00030  72 69 70 74 69 6f 6e 73  2e 20 42 79 20 64 65 66  riptions. By def |
 | 00040  61 75 6c 74 2c 0a 6e 63  75 72 73 65 73 20 77 69  ault,.ncurses wi |
 | 00050  6c 6c 20 73 65 61 72 63  68 20 24 7b 48 4f 4d 45  ll search ${HOME |
 | 00060  7d 2f 2e 74 65 72 6d 69  6e 66 6f 20 66 69 72 73  }/.terminfo firs |
 | 00070  74 2c 20 74 68 65 6e 20  2f 65 74 63 2f 74 65 72  t, then /etc/ter |
 | 00080  6d 69 6e 66 6f 20 28 74  68 69 73 0a 64 69 72 65  minfo (this.dire |
 | 00090  63 74 6f 72 79 29 2c 20  74 68 65 6e 20 2f 6c 69  ctory), then /li |
 | 000a0  62 2f 74 65 72 6d 69 6e  66 6f 2c 20 61 6e 64 20  b/terminfo, and  |
 | 000b0  6c 61 73 74 20 6e 6f 74  20 6c 65 61 73 74 20 2f  last not least / |
 | 000c0  75 73 72 2f 73 68 61 72  65 2f 74 65 72 6d 69 6e  usr/share/termin |
 | 000d0  66 6f 2e 0a 1b 5b 37 6d  2f 65 74 63 2f 74 65 72  fo...[7m/etc/ter |
 | 000e0  6d 69 6e 66 6f 2f 52 45  41 44 4d 45 20 28 45 4e  minfo/README (EN |
 | 000f0  44 29 1b 5b 32 37 6d 1b  5b 4b                    D).[27m.[K       |

@gwsw
Copy link
Owner

gwsw commented Dec 22, 2020

Looks like the tty driver is failing to set raw mode. At 22:27:19.536470 less opens /dev/tty and a few lines later calls TCSETSW to enable raw mode. But the behavior you describe sounds like the terminal is still in cooked mode, and is waiting for a newline before processing input characters.

@al20878
Copy link
Author

al20878 commented Dec 22, 2020

I pressed Enter in between the qs so there were a few newlines in there... Everything gets processed later on when I kill less: all those typed (and echoed!) characters interpreted by the shell as commands.

@al20878
Copy link
Author

al20878 commented Dec 22, 2020

Also, shell understands and interprets keyboard arrows perfectly fine (e.g. command recalls with up and down, as well and editing with left and right), meaning the terminal driver is able to switch to raw mode. I don't understand, however, why less needs the (reopened) terminal at fd 3, and why it can't use fd 0, which apparently works alright (both shell and more just support that).

@gwsw
Copy link
Owner

gwsw commented Dec 22, 2020

Less doesn't use fd 0 because if you pipe a file into less, fd 0 is not the terminal. So less uses /dev/tty if it exists, otherwise fd 2 which is usually the terminal but not always readable on all systems.

What OS are you using? I think some older Linux versions had bugs in the /dev/tty implementation. I guess you could temporarily rename /dev/tty to force less to use fd 2 and see if that behaves differently.

@al20878
Copy link
Author

al20878 commented Dec 22, 2020

Less doesn't use fd 0 because if you pipe a file into less, fd 0 is not the terminal

Indeed. But, well, less could have used isatty() on fd 0 to see whether that was the case, or any other TTY-specific ioctl() and check the return code (and errno)... If it still must use a distinct fd later on, there's dup() and no need to open it from scratch in the case of fd 0 being already a TTY. Just a suggestion...

What OS are you using?

It's buster (Raspberry Pi OS, 32-bit Linux), Debian/Ubuntu, basically :-) All updates current

@gwsw
Copy link
Owner

gwsw commented Dec 22, 2020

Hm, well this has not been reported previously so I suspect there's something in your environment that is causing less to malfunction. I'd like to see if the behavior is different when using fd 0 vs. using /dev/tty. So could you either rename /dev/tty as I suggested above, or just change line 69 in ttyin.c from
tty = open("/dev/tty", OPEN_READ);
to
tty = 0;
and see if the behavior changes?

@al20878
Copy link
Author

al20878 commented Dec 23, 2020

I replaced it like this:

       tty = dup(0);//open("/dev/tty", OPEN_READ);

and was able to control less the usual way: all the key commands appear to be working correctly, and (hooraaaay!) q quits it!

BTW, I checked that replacing the dup(0) above with just 0 (as you asked) also makes the keys work fine.

@al20878
Copy link
Author

al20878 commented Dec 23, 2020

Like I noted previously, more and shell command line editing with arrows all work just out of the box...

@gwsw
Copy link
Owner

gwsw commented Dec 23, 2020

Ok, so unfortunately this points to there being a bug in your /dev/tty driver. When stdin is connected to the terminal, fd 0 and /dev/tty should behave the same, but clearly they don't on your system. I might be tempted to just remove the /dev/tty node from your system since it doesn't work correctly, but that might break other programs that don't depend on /dev/tty supporting raw mode.

@al20878
Copy link
Author

al20878 commented Dec 23, 2020

Well, thanks, but unfortunately, that is not "my" system -- that's what I use. But for less to be more adaptable to different peculiarities, it may be smart/prudent to include some code that would dup stdin (rather than re-opening "/dev/tty") in cases when stdin is already a TTY -- it's way faster too (not that it matters a lot for an interactive application). Again, just a suggestion... And sorry for keeping saying that, but somehow more and screen text editors are able to work on "my" system just fine...

@gwsw
Copy link
Owner

gwsw commented Dec 23, 2020

Less could be changed to dup stdin instead of opening /dev/tty, but why? If the purpose is just to workaround systems which have broken /dev/tty implementations, that raises the question of what less should do on such a system if stdin is NOT a terminal. A version of less which doesn't support input pipes would of course be unacceptable. It would also only work on systems which have the dup() system call; not all systems supported by less do so. I don't know what more is doing on your system; I guess you could use strace to find out, but it may be using some mechanism specific to Linux, which I try to avoid. In general, I'm reluctant to change code that has existed for over 20 years without any problems, merely to support a broken OS.

@al20878
Copy link
Author

al20878 commented Dec 23, 2020

that raises the question of what less should do on such a system if stdin is NOT a terminal

Fall back to the same logic that existed for 20 years: open "/dev/tty"; or, if that fails, resort to using stderr

It would also only work on systems which have the dup() system cal; lnot all systems supported by less do so.

Sure. Configure can take care of that. But TBH, I haven't heard of a Un*x without a dup() syscall. That branch of code in ttyin.c (line 74, in particular) is extremely Un*x-specific: after all, it tries to open /dev/tty. But #ifdef HAVE_DUP or something like that, should cover it, no?

I guess you could use strace to find out

I did! And it was all attached to the very top of this thread.

I'm reluctant to change code that has existed for over 20 years without any problems, merely to support a broken OS

Well, that is called being flexible. And that is why there's configure and all those conditionals -- to support various OSes/compilers, some features of which may seem like "broken" but the code works around them to present a functional solution.

Again, I was just making a suggestion. Up to you to decide, obviously. I know how to rebuild less on "my" system to make it working. I appreciate that

@gwsw
Copy link
Owner

gwsw commented Mar 4, 2021

Probably 4d4e6f4 helps with this too.

@gwsw gwsw closed this as completed Mar 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants