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

Core: Use log variant of PrintCallstack, not print variant #10159

Merged
merged 1 commit into from Oct 12, 2021

Conversation

JosJuice
Copy link
Member

There are two reasons for this.

  1. Using Dolphin's logging system lets the user decide whether the printout should go to the terminal, the GUI, or a file. fmt::print always prints to stdout... unless you're on Android, in which case it does nothing at all, because Android disables stdout.

  2. The Windows version of Dolphin crashes when you use fmt::print. Yes, really. The crash happens because a call to std::fprint in fmt::v7::detail::fwrite_fully returns that less characters were written than requested, which fmt handles by throwing an exception. (As always, Dolphin does not use exception handling.) I'm not sure why std::fprint is doing this, but since switching away from using fmt::print is a good idea due to the previous point anyway, I'd say it's best to just switch.

There are two reasons for this.

1. Using Dolphin's logging system lets the user decide whether
the printout should go to the terminal, the GUI, or a file.
fmt::print always prints to stdout... unless you're on Android, in
which case it does nothing at all, because Android disables stdout.

2. The Windows version of Dolphin crashes when you use fmt::print.
Yes, really. The crash happens because a call to std::fprint in
fmt::v7::detail::fwrite_fully returns that less characters were
written than requested, which fmt handles by throwing an exception.
(As always, Dolphin does not use exception handling.)
I'm not sure why std::fprint is doing this, but since switching
away from using fmt::print is a good idea due to the previous point
anyway, I'd say it's best to just switch.
@Adamillo
Copy link

Can confirm that this PR fixes the crash

@leoetlino leoetlino merged commit 6c1a625 into dolphin-emu:master Oct 12, 2021
10 checks passed
@JosJuice JosJuice deleted the print-callstack-log branch October 13, 2021 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants