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: Allow PCAP shared read access on Windows #11089

Merged
merged 1 commit into from Jan 5, 2023

Conversation

sepalani
Copy link
Contributor

This PR allows shared read access of PCAP file being dumped so we don't have to wait the dump to be finished (i.e. shutdown the game) to read it on Windows.

Ready to be reviewed & merged.

@@ -62,12 +63,20 @@ void IOFile::Swap(IOFile& other) noexcept
std::swap(m_good, other.m_good);
}

bool IOFile::Open(const std::string& filename, const char openmode[])
bool IOFile::Open(const std::string& filename, const char openmode[], SharedAccess sh)
Copy link
Member

@lioncash lioncash Sep 29, 2022

Choose a reason for hiding this comment

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

Should this perhaps log anything out on non-windows systems that the passed in shared access will be ignored?

Maybe sh should be tagged as [[maybe_unused]] as well

Aside from that one concern, everything looks good to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should this perhaps log anything out on non-windows systems that the passed in shared access will be ignored?

It won't be necessary since it's the default behaviour on the other systems (i.e. Default == Read). I can already access the PCAP file from Linux and Android builds since fopen/fdopen don't lock the file. Unlike Windows' fopen_s that doesn't allow share read.

Maybe sh should be tagged as [[maybe_unused]] as well

Done.

@AdmiralCurtiss AdmiralCurtiss merged commit 7b04a6b into dolphin-emu:master Jan 5, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants