Atmosphere-NX / Atmosphere

Atmosphère is a work-in-progress customized firmware for the Nintendo Switch.
GNU General Public License v2.0
14.13k stars 1.21k forks source link

dmnt.gen2: More logging in GDB #2288

Open MonsterDruide1 opened 4 months ago

MonsterDruide1 commented 4 months ago

There are a lot of things that can go wrong when dealing with GDB, and most of them are caught by Atmosphere in a few checks before actually running "dangerous" stuff. However, when actually running into an issue, Atmosphere currently just sends back an error message without further explanation. As the GDB protocol doesn't intend to include error messages, these should at least be logged to the logfile, if logging to a file is enabled.

This PR goes through the whole GDB implementation, and attaches a message to every E01 response, clarifying the cause of the issue (as far as it's known). This will help in debugging further issues with GDB and its protocol.

Side note: This PR contains quite simple changes, so I can get to know the process of contributing to Atmosphere. I did already implement some changes to make the multiprocess+ optional for working with older/simpler GDB clients, which I will clean up and PR once this is in.

SciresM commented 4 months ago

Well, I am on vacation in Japan until March 25th.

That aside, step 1 for contributing to atmosphere is "reach out to me on discord" to talk about whatever you're looking to contribute. I generally don't accept unsolicited pull requests...

...in this case, I don't think that logging outside of debug configurations is actually desirable.

I think I would want this ifdef'd to continue being a null-operation under default build config.

Anyway the actual commentary is that you don't seem to understand atmosphere's coding style - you have a variable namedLikeThis, but atmosphere exclusively uses named_like_this for local variables.

MonsterDruide1 commented 4 months ago

Alright, I didn't find any contribution or styling guide regarding this project, so I expected getting something wrong - thanks for clarifying.

I can't contact you/write my intentions on the ReSwitched Discord server, because I'm blocked from writing in any major channels - only #dev-support would be open, but doesn't quite match the topic, as I don't need help with it. Should I DM you instead?

This logging is not done "normally", only if the debugging for GDB is explicitly enabled.

Also, I adjusted the name to should_log_result - thanks for the comment.

SciresM commented 4 months ago

Yes, #dev-support would be the correct channel, though usually I just have people DM me.

Actually reviewing the logs...I am not really sure I want these in? A lot of them are clarifying cases that should never occur using devkitPro's gdb, which is the only guaranteed-supported client to the stub.

Also, why log reply/response - isn't that the purpose of gdb's "set debug remote 1"?

MonsterDruide1 commented 4 months ago

Alright, let's continue this in DMs then, if that's what you prefer!