fix: Missing --help output & other diagnostic messages in console

[csnover]

Problem description

Typing --help causes ImHex to exit without outputting anything. Diagnostic messages from glib, ASan, other libraries that might have something important to say, etc. are also suppressed.

Implementation description

This effectively reverts 7c1e33d, which was partially reverted only on Windows by code that was left commented out in f114239.

Allowing other libraries to print to stderr may make the output ‘ugly’, but lots of things print to stderr that are important for figuring out why something is bugged, like ASan and glib.

Additional things

[WerWolv]

The reason why I turned these off was because libmagic just spammed hundreds of lines of bogus log messages to stderr without any way to turn it off. That's just really terrible and outdated library design.

I'd argue that we should keep this for the sake of usability of the logs and instead only disable stderr in release builds where you won't be using diagnostics tools or asserts anyways.

[csnover]

Do you know where in libmagic this is happening, or how I could trigger it? Looking at the source code it seems like debugging output is gated behind MAGIC_DEBUG flags and I don’t remember file ever emitting messages.

[WerWolv]

When loading compiled magic files or compiling some magic source files, it would spew out hundreds of warning / error logs to the console. MAGIC_DEBUG isn't enabled anywhere in ImHex to my knowledge

[csnover]

Oops. I see MAGIC_CHECK flag is being used. Emitting diagnostic messages to stderr is also its intended purpose according to magic.h:

#define	MAGIC_CHECK		0x0000040 /* Print warnings to stderr */

There are a couple of other places where messages look like they are unconditionally emitted via file_magwarn in libmagic when there is an error with the magic file but certainly that would not seem like a thing that would flood the stderr like MAGIC_CHECK (which is the flag used for file -c) does.

To me it seems like the thing to do for libmagic specifically could be to temporarily redirect stderr to collect the emitted warnings while it is running rather than suppressing all stderr output globally. Presumably if someone is using a custom magic file and there is some syntax error it would be helpful for them to see that; getting a single error from magic_error seems like it would be less useful. But, I don’t know how people are using this feature in practice, so maybe just stopping asking libmagic to emit warnings to stderr is the thing to do.

--help should emit regardless of debug or release mode; it is also possible to run release version of ImHex with e.g. debug libraries dynamically linked that emit diagnostic information to stderr, so I don’t know that it is good enough only suppress the output on a release version. It may be worthwhile to unconditionally redirect stderr messages into the normal logging system, though, so they are not absent from the log window.

[WerWolv]

Hm that flag should definitely be removed. But since it still doesn't silent all errors it's not really a full solution.

Personally, I just believe that some random system library has no right to dump a bunch of crap into my program. That log output is not integrated into the whole logging system at all so first of all it doesn't even show up in the in-app log console and second it's generally not helpful to the end user. People aren't using ImHex to debug or write magic files normally.

What I find weird though is why --help doesn't show up in the console for you. It definitely does for me and it uses the default logging mechanism through stdout to print its output, ImHex never uses stderr

[csnover]

I agree that libmagic seems like a mess of a thing that probably started its life as an executable and only got half-abstracted out into a library. I am no fan of C—especially error handling in C libraries, which is almost universally awful—and also agree that in an ideal world things would not be this way. Unfortunately, the standard mechanism for emitting most diagnostic information today is still just writing to stderr. So, when there is a conflict between what an application author wants and what the end user or platform convention wants, who gets to win? In such cases, I try to think about the benefits versus harms to users. In this case, assuming ImHex is fixed to not ask libmagic to write a lot of stuff to stderr, I see it this way:

What benefit is there to users to suppress stderr? If a user is not looking at the terminal output, their experience is unchanged. If they have some OCD, they will be less upset that message formatting in the console is not uniform. If irrelevant diagnostics are emitted, they will not be alarmed or misled by their existence. They will not need to be aware of their own ability to redirect or filter stderr to suppress unwanted messages from libraries.

What harm is there to users to suppress stderr? It breaks platform convention and creates confusion for users who expect stderr to behave in the standard way. Errors from libraries that cannot unwind the stack, but that emit to stderr before calling abort, will be missing, which makes crashes harder to understand and troubleshoot. Other non-fatal diagnostic information potentially relevant to an issue will be discarded. Environment variables like WAYLAND_DEBUG=1, which are relevant to both debug and release builds of any application, stop working. Some (hypothetical) cli plugins that might use stderr for diagnostics and stdout for output will be impossible to author/use as ImHex suppresses stderr and emits its own diagnostics to stdout.

As far as why --help was not working: I am not sure of the answer to this. vscode’s built-in terminal may have been doing something weird (vscode’s terminal has definitely broken other things before in weird ways), or maybe I was wrong about what happened and ImHex was crashing before it was able to emit the help output at all. There are some nuances of stdio that are beyond my knowledge. At least the lack of --help output was what clued me in to check if ImHex was messing with stderr, since some apps emit help on stderr.

Originally, ImHex was crashing due to a memory error, and glibc was emitting a diagnostic, but ImHex was suppressing it. I occasionally have three brain cells to rub together and eventually ran strace which showed the write call containing part of the suppressed error. (Attaching a debugger is not good enough AFAIK, as it will only trap on the abort, at which point the frame with the diagnostic message will usually be gone.)

So, these are my thoughts. Maybe you have a different idea about which things are most important and that’s fine, do whatever you think is best. This did cause me to waste time and energy and I like to try to make it so others don’t have to waste time and energy too.

[csnover]

Probably neither here nor there at this point but vscode’s built-in terminal does seem to do something weird with file descriptors that other terminals don’t that seems to be able to cause applications to break. I tried to run flatpak wine inside of a vscode terminal, which works fine in a normal terminal, but hangs with 100% CPU forever in vscode. strace shows fnctl(<monotonically increasing number>, F_SETFD, FD_CLOEXEC) = -1 EBADF. I don’t plan on debugging this further but just figured I would drop a note to say I encountered some other dumb broken vscode terminal thing about FDs.