On Tue, Dec 12, 2023 at 11:09:13PM +0100, наб wrote: > Package: libncursesw6 > Version: 6.4+20231121-1 > Severity: normal > > Dear Maintainer, > > urlview 1c-1 whose xterm was killed but which didn't die consumes 100% CPU. > The event loop is for(;;) switch(getch()) ... and ignores -1
If it's going to ignore the error return (while at the same time manipulating the time delays with mousemask), then this is expected (mis)behavior. That's in the manpage -- see below. > (https://git.sr.ht/~nabijaczleweli/urlview-ng/tree/1c/item/urlview.c#L576). > > strace -p: > read(0, "", 1) = 0 > read(0, "", 1) = 0 > read(0, "", 1) = 0 > read(0, "", 1) = 0 > read(0, "", 1) = 0 > read(0, "", 1) = 0 > &c. > > ltrace -p: > wgetch(0x55e356e14860, 0, 0, 1) = 0xffffffff > wgetch(0x55e356e14860, 0xffffffff, 0, 0xffffffff) = 0xffffffff > wgetch(0x55e356e14860, 0, 0, 1) = 0xffffffff > wgetch(0x55e356e14860, 0xffffffff, 0, 0xffffffff) = 0xffffffff > wgetch(0x55e356e14860, 0, 0, 1) = 0xffffffff > wgetch(0x55e356e14860, 0xffffffff, 0, 0xffffffff) = 0xffffffff > wgetch(0x55e356e14860, 0, 0, 1) = 0xffffffff > wgetch(0x55e356e14860, 0xffffffff, 0, 0xffffffff) = 0xffffffff > &c. > > gdb -p, b ./urlview.c:576: > (gdb) cont > Continuing. > > Breakpoint 1, main (argc=<optimized out>, argv=<optimized out>) at > ./urlview.c:576 > 576 int c = getch(); > (gdb) n > 578 switch(c) { > (gdb) p c > $1 = -1 > (gdb) p *__errno_location() > $3 = 5 > this is EIO. By resetting *__errno_location() = 0 and letting it rip for > a few more loops, I see it continue to be 0, > so it just returns 0 but doesn't change errno > (I'm assuming changing errno is a side-effect of read(), > which completes successfully but emptily here), > presumably the EIO is from an earlier refresh to a dead pty. > > getch(3ncurses) on bookworm and sid says > RETURN VALUE > All routines return the integer ERR upon failure and an integer > value other than ERR (OK in the case of ungetch) upon successful > completion. > > wgetch returns ERR if the window pointer is null, or if its > timeout expires without having any data, or if the execution ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > was interrupted by a signal (errno will be set to EINTR). > so how precisely you're meant to handle this is unclear to me. > > The first sentence implies "case ERR: exit(1)", > but the second implies "case ERR: if(errno != EINTR) exit(1); break" > to me. > > I set no timeouts so I expect that bit to not apply, > but I read this as the EINTR parenthetical applying to both the > interruption and timeout cases. > > Either way, if errno isn't (re)set, then returning ERR isn't really > reliable or meaningful i think (lest you "errno=0, getch()" always? > which kinda sucks). > > I see SUSv2 XCURSES say: > Upon successful completion getch(), mvgetch(), mvwgetch() and > wgetch() return the single-byte character, KEY_ value, or ERR. > When in the nodelay mode and no data is available, ERR is returned. > which makes it silent on the issue. > > Reopening its pty fails with EIO, as expected, but consulting stty > against urlview in a live xterm I see > speed 38400 baud; rows 54; cols 172; line = 0; > -brkint -icrnl -imaxbel iutf8 > -onlcr > -icanon -echo > so -icanon min=1 time=0, which is by no means a "nodelay" mode, > and it doesn't have a "timeout" set. > > Naturally, setting min=0 time=1 I reproduce the empty-returning read()s, > and the unchanging errno. > > So to this point, I think this just needs disambiguation to the tune of > "having any data (with errno unchanged)". > > Empty read()s from a hung-up teletype aren't covered by the > documentation. Naturally this is an adversarial reading, > but it may not be obvious to all readers that these are homologous > conditions: > "or if the read from returns empty \(em be it due to a timeout > expiring with no data or due to a hangup (with errno unchanged)," > > In a similar vein, I see a lot of references to a "cbreak mode" > (sometimes in bold) and a "nocbreak mode" (never in bold). > I suppose this is a left-over from old berkeley manuals(?), > but that's not a feature of the Linux teletype driver, > and according to my notes[1] the CBREAK mode features in [V7, 4.4BSD) > exclusively, which isn't really useful to the modern reader, > with the 4.4BSD URM being released close to 30 years ago now. > The only remnant of this remains in stty(1), but that consistently calls > it cbreak/-cbreak mapping it to -icanon/icanon. This makes it even more > jarring to a modern reader. > > Anyway, is this behaviour expected, and is the loop best-served as > for(;;) switch(errno=0, getch()) case ERR: if(errno != EINTR) { err = true; > break 2; } > ? Or is there a better way to drive this? > > Best, > наб > > [1]: > https://lfs.nabijaczleweli.xyz/0012-groff-mdoc-*q-spacing/2022-10-23-stty.1-preprint/a4.pdf > pp. 20, 67-68 > > -- System Information: > Debian Release: 12.2 > APT prefers stable-updates > APT policy: (500, 'stable-updates'), (500, 'stable-security'), (500, > 'stable-debug'), (500, 'stable') > Architecture: amd64 (x86_64) > Foreign Architectures: i386 > > Kernel: Linux 6.1.0-9-amd64 (SMP w/24 CPU threads; PREEMPT) > Kernel taint flags: TAINT_PROPRIETARY_MODULE, TAINT_FIRMWARE_WORKAROUND, > TAINT_OOT_MODULE, TAINT_UNSIGNED_MODULE > Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8), > LANGUAGE=en_GB:en > Shell: /bin/sh linked to /usr/bin/dash > Init: systemd (via /run/systemd/system) > LSM: AppArmor: enabled > > Versions of packages libncursesw6 depends on: > ii libc6 2.36-9+deb12u3 > ii libtinfo6 6.4+20231121-1 > > Versions of packages libncursesw6 recommends: > ii libgpm2 1.20.7-10+b1 > > libncursesw6 suggests no packages. > > -- no debconf information -- Thomas E. Dickey <dic...@invisible-island.net> https://invisible-island.net
signature.asc
Description: PGP signature