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

Update DS2_ReplaceServerAddressHook.cpp #220

Merged
merged 5 commits into from
Jul 23, 2024
Merged

Conversation

TroncoNinja
Copy link
Contributor

fix for wine check in ds2, if you are running in wine(proton) ignore check for memory safety features that aren't supported.
the injector file compiled in this version was also tested with the 41.0 version (currently working on linux for the official unofficial server only)

fix for wine check in ds2, if you are running in wine(proton) ignore check for memory safety features that aren't supported
@NoyingNabi
Copy link

This is great, you are goated! Hopefully some more people can test it, I could not get it to build.

Copy link
Owner

Choose a reason for hiding this comment

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

Whats the reason for this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my bad, it was only for running in my GitHub action to test the package, didn't mean to commit this, i will re-add the code

{
continue;
//if i'm not running in wine do the check
if (winePrefix == nullptr) {
Copy link
Owner

Choose a reason for hiding this comment

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

couple of coding style issues:

  • braces go on next line
  • comments start with a space and a capital letter
  • variables are in PascalCase not camelCase

continue;
//if i'm not running in wine do the check
if (winePrefix == nullptr) {
Log("you are using windows");
Copy link
Owner

Choose a reason for hiding this comment

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

Can you remove this please, it isn't needed.

if (VirtualQuery((void*)key, &info, sizeof(info)) == 0 ||
((info.Protect & PAGE_READWRITE) == 0 && (info.Protect & PAGE_EXECUTE_READWRITE) == 0))
// If the programm is running on wine
if(winePrefix != nullptr)
Copy link
Owner

Choose a reason for hiding this comment

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

space after if please, also the comment doesn't really add anything, please remove it.

{
continue;
Log("you are using wine");
Copy link
Owner

Choose a reason for hiding this comment

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

Remove the log please.

{
continue;
Log("you are using wine");
if (VirtualQuery((void*)key, &info, sizeof(info)) == 0) //wine doesn't emulate memory seafe features of windows {il TroncoNinja e' stato qui}
Copy link
Owner

Choose a reason for hiding this comment

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

Comment on previous line please - also please stick to english, even if its just a fun tag :)

{
continue;
Log("you are using wine");
if (VirtualQuery((void*)key, &info, sizeof(info)) == 0) //wine doesn't emulate memory seafe features of windows {il TroncoNinja e' stato qui}
Copy link
Owner

Choose a reason for hiding this comment

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

I'm curious, what does wine actually return for the protection flags? I would expect it to return the same values as windows, as I can see a whole slew of compatibility issues if that wasn't the case.

Copy link
Contributor Author

@TroncoNinja TroncoNinja Jul 10, 2024

Choose a reason for hiding this comment

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

the Flags are all correct, as far as my testing goes, the problem is in the info struct, more specifically the changes made in the info.Protect (in windows info.Protect=64, in wine/proton info.Protect = 128). The memory seems to have the PAGE_EXECUTE_WRITECOPY flag instead of PAGE_EXECUTE_READWRITE. the flags for memory safety in linux are different, as far as my research goes, wine has problems(design) translating the flags/translating memory managment

@TLeonardUK
Copy link
Owner

Thanks for the PR!

I left a handful of minor style comments, if you could fix those I'll happily merge this.

@TroncoNinja TroncoNinja requested a review from TLeonardUK July 10, 2024 11:49
@TLeonardUK TLeonardUK merged commit d28fb82 into TLeonardUK:main Jul 23, 2024
4 checks passed
@TLeonardUK
Copy link
Owner

Merged in the pull request, sorry for the delay!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants