lib/tty: add proper error handling for tcgetattr()

[mukhanov]

• Check if stdin is a terminal before calling tcgetattr()
• Verify return value of tcgetattr() and handle errors properly
• Exit with clear error message when stdin is not a terminal
• Prevents mc from hanging when run in non-interactive environments

This fixes the issue where mc would hang on startup when stdin is not
a terminal (e.g., when run in background or in certain terminal emulators
on macOS). The program now exits with a clear error message instead of
hanging or exhibiting undefined behavior.

[mc-worker]

Treating !isatty (fileno (stdin)) as an error makes impossible an implementation of #2370 and #2629 in the future.

Btw, tty_init() in the lib/tty/tty-ncurses.c calls tcgetattr() without return value checking too.

[mukhanov]

Thank you for the feedback!

I've updated the fix to:

1. Only treat !isatty() as an error for MC_RUN_FULL mode
2. For MC_RUN_VIEWER and MC_RUN_EDITOR modes, tcgetattr() failures are handled gracefully by initializing the mode structure with default values
3. Also fixed the same issue in lib/tty/tty-ncurses.c

This approach:

• Prevents mc from hanging when run in non-interactive environments (the original issue)
• Allows future implementation of Allow mcview to display from stdin #2370 and Allow editor to read data from stdin #2629 (cat file | mcview/mcedit)
• Properly handles tcgetattr() errors in both slang and ncurses backends

The changes ensure that viewer/editor modes can work with pipes while full mc mode still requires a proper terminal.

[zyv]

@mukhanov - thanks for your submission. I've screened it through Opus 5.5, and it has identified a number of issues and suggests a different approach. Would you be up to reviewing and testing the suggested alternative? I can submit the results later this coming week.

[mukhanov]

Hi @zyv,

Yes, I'd be happy to review and test the alternative approach suggested by Opus 5.5.

This is indeed a rather annoying bug - mc hanging on startup when stdin is not a terminal is a real problem that needs a proper fix. I'm particularly interested in seeing how Opus 5.5 handles the error cases differently from my initial approach.

Looking forward to reviewing the results when you have them ready.

Best regards

[zyv]

Okay, I have finally managed to process the alternative patch series generated with Opus 5.5, which do make sense to me. It includes a test that passes with the suggested changes and fails with your approach, and I think it addresses both points by @mc-worker. I've added the edited LLM review summary below for the details. Appreciate it if you could review and test it @mukhanov and report back.

---

The underlying problem is real, but the PR addresses it at the wrong level, and every run test reproduces at least one damaging behaviour:

• It breaks a working use case on the default S-Lang backend. mc < /dev/null and ... | xargs mc work on master and now fail (F1).
• It leaves the user's terminal without echo or line editing when the new error exits are taken on ncurses (F2).
• It does not fix the hang it describes for mcview / mcedit / mcdiff on ncurses. They still spin at 100 % CPU on EOF or ignore the keyboard on a pipe (F3).
• It codifies the zero-filled boot_mode as "default values". That mode is applied to the real terminal before every external command: speed 0, no ISIG (Ctrl-C cannot stop the command), no echo (F4, pre-existing).

Right fix: mc never reads data from stdin, but it reads the keyboard from stdin with ncurses, and both backends save the initial terminal modes from stdin. So the correct fix is to attach a redirected stdin to the controlling terminal once, in main(), before any terminal set-up, as xargs -o does ¹. Fall back to stderr as S-Lang does ², and fail with a clear message only when there is no terminal at all. This:

• fixes the hang on both backends;
• keeps the S-Lang use cases working;
• fixes the zeroed boot_mode;
• keeps stdin reading possible - that feature would dup() the original stdin first:
  • Allow mcview to display from stdin #2370
  • Allow editor to read data from stdin #2629

---

#	Severity	Finding
F1	High	FULL mode now refuses to start with redirected stdin on S-Lang (the default backend), although S-Lang reads keys from /dev/tty and this worked
F2	High	Zero-filled boot_mode is applied to the real terminal before every external command (speed 0, -isig -icanon -echo -opost). Pre-existing, but the PR rewrites this code and documents it as safe. Run test (stty -a from user menu).
F3	High	The hang the PR targets remains for viewer, editor and diff viewer on ncurses (100 % CPU on EOF, keyboard ignored on a pipe). The "stdin may be a pipe, e.g. cat file | mcview" premise is not a feature mc has
F4	Medium	ncurses: both new exit() paths run after initscr() without endwin(), leaving the shell with -echo -icanon -onlcr
F5	Low	After a failed tcgetattr() the zero-filled struct is still written with tcsetattr() (ncurses). The S-Lang new_mode check tests the wrong call: SLang_init_tty()'s result is ignored
F6	Low	exit() from inside tty_init() bypasses main()'s startup cleanup and error format ("Failed to run: ...")
F7	Low	Same 15-line block pasted 3 times; two new translatable strings in a non-standard format; redundant #include <string.h>; errno used without <errno.h>

Footnotes

1. GNU findutils 4.9.0 xargs --help output (-o, --open-tty) and local run (echo x | xargs sh -c 'readlink /proc/$$/fd/0' → /dev/null). Local tool output, Ubuntu 24.04 container. ↩

2. [8] S-Lang 2.3.3 source, src/slutty.c (SLang_init_tty(), lines 276-321). Ubuntu archive upstream tarball. ↩

[zyv]

P.S. I can look into the Solaris issue when everybody is satisfied with the approach in principle.