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

Windows: Old "with admin priviledges" version is broken #112

Open
dreua opened this issue May 17, 2024 · 2 comments
Open

Windows: Old "with admin priviledges" version is broken #112

dreua opened this issue May 17, 2024 · 2 comments

Comments

@dreua
Copy link
Member

dreua commented May 17, 2024

  • On my win11 machine:
    • the default build with no admin privs does work as expected
    • the build with admin privs does not work. It only shows lost pings but with wireshark i could not see any icmp packets beeing sent. So that does not seem to work. I added some debug prints and sendto returns an expected value of 20. My windows debugging skills are at my limit there.

[...]

Even tho the version with admin privileges is broken i believe this PR is "completed" as on the current master this doesn't even compile. So i consider that feature "not made worse" 😄

Originally posted by @mrbesen in #109 (comment)

I just wanted to have this documented, there is probably no real use in the "with admin prives" version anymore. Maybe we could consider removing the code and making the no admin version the only available.

@dreua dreua changed the title Broken on windows with admin priviledges Windows: Old "with admin priviledges" version is broken May 17, 2024
@mrbesen
Copy link
Contributor

mrbesen commented May 17, 2024

Before we do remove that code, did someone other than me checked if it works? I have only tested it on one single machine, that might have some other (Group Policy) reasons for this to not work. Like i said, my windows debugging skills are limited.

But anyways:
I am in favor of removing that code, as that is quite a lot of code, that is

  • broken
  • duplicating a feature (ping on windows)
  • making the cnping src harder to read / understand
  • adding a compile flag

@cnlohr
Copy link
Member

cnlohr commented May 24, 2024

Is there a recommended avenue forward, @dreua ?

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

No branches or pull requests

3 participants