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

Attachment: signature.asc
Description: PGP signature

Reply via email to