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

NetworkCaptureLogger: Move SSL logging #5978

Merged
merged 2 commits into from Jan 28, 2021
Merged

Conversation

sepalani
Copy link
Contributor

@sepalani sepalani commented Aug 24, 2017

This PR moves the SSL logging part to its own logger class. Not sure, if putting it inside SConfig is a good idea so I'm open to suggestions regarding where it should belong.

Ready to be reviewed & merged.

@sepalani sepalani changed the title NetworkCaptureLogger: Move SSL Logging NetworkCaptureLogger: Move SSL logging Aug 24, 2017
@sepalani sepalani force-pushed the net-log branch 3 times, most recently from 9eaa694 to 82a472c Compare August 24, 2017 19:22
@sepalani sepalani force-pushed the net-log branch 4 times, most recently from a00d4cf to 4a24d35 Compare August 29, 2017 21:59
@delroth
Copy link
Member

delroth commented Sep 2, 2017

@dolphin-emu-bot rebuild

@sepalani
Copy link
Contributor Author

Rebased on latest master.

Still ready to be reviewed & merged.

Copy link
Member

@BhaaLseN BhaaLseN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM I guess. Took me a moment to realize IOFile::Write does nothing if the file isn't open, but relying on that keeps the code simple though.

@sepalani
Copy link
Contributor Author

sepalani commented Mar 2, 2019

Rebased on latest master.

@sepalani sepalani force-pushed the net-log branch 2 times, most recently from 0fb8656 to 0144250 Compare May 15, 2020 18:51
@sepalani
Copy link
Contributor Author

Rebased on #8812.

I slightly changed the logger interface and moved the logger to PowerPC::debug_interface rather than SConfig.

@sepalani
Copy link
Contributor Author

Rebased on latest master, I deleted the copy/move constructors and assignment operators as suggested #6075 (comment). I also addressed the following comments: #6075 (comment) & #6075 (comment).

Still ready to be reviewed & merged.

@sepalani
Copy link
Contributor Author

Rebased on latest master.

Still ready to be reviewed & merged.

Copy link
Contributor

@iwubcode iwubcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (untested)

@@ -534,6 +534,7 @@ static void EmuThread(std::unique_ptr<BootParameters> boot, WindowSystemInfo wsi

PatchEngine::Shutdown();
HLE::Clear();
PowerPC::debug_interface.Clear();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right after I hit the approve button, I realized I glossed over this. What's this for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It clears the debug_interface after a full shutdown (i.e. breakpoints, patches, watches, memcheck) and it also releases the network logger and its file.

I have the feeling it's an artifact that wasn't migrated when switching to Qt, I might be wrong but I don't recall adding such line when I originally created the PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok, looks like this might also resolve #9325 and the issue that was mentioned there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, my wording was poorly chosen. By "full shutdown" I meant the game/Wii shutdown not the actual game so it won't.

After a closer look at the current code, I'll probably add a 2nd commit that removes the GUI code that takes care of the breakpoints/memchecks clearing as it's using Qt signals and Dolphin state rather than removing them on shutdown in core.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary GUI code removed in the 2nd commit.

PPCDebugInterface.Clear() is called by Core on shutdown instead
@leoetlino leoetlino merged commit 906fbf6 into dolphin-emu:master Jan 28, 2021
10 checks passed
@sepalani sepalani deleted the net-log branch January 29, 2021 03:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
7 participants