Skip to content

Use gethostname on Windows#4

Closed
Jake-Shadle wants to merge 8 commits intodjc:masterfrom
Jake-Shadle:master
Closed

Use gethostname on Windows#4
Jake-Shadle wants to merge 8 commits intodjc:masterfrom
Jake-Shadle:master

Conversation

@Jake-Shadle
Copy link
Copy Markdown

The Windows implementation was using GetComputerName even though
gethostname is available on Windows, so I updated to the latest winapi
instead of winutil.

Also added CI.

@Jake-Shadle
Copy link
Copy Markdown
Author

This should also fix #3 as it removes the winutil dependency and therefore the loose dependency on winapi in favor of a more strict one.

@fengcen
Copy link
Copy Markdown
Contributor

fengcen commented Dec 20, 2017

Thanks.

I found that these changes will cause performance problems:

Before:
before

After:
after

So I think there is no need to change. Thanks anyway.

@Jake-Shadle
Copy link
Copy Markdown
Author

Yah, I didn't run the benchmark, my bad!

I've updated to use GetComputerNameExA instead, so I get
image

Which is roughly 5 times as expensive as the previous version on my machine, but should be more accurate as the function winutil is calling is capped at 16 characters (including null terminator) while this one can represent the full hostname.

@swsnr
Copy link
Copy Markdown

swsnr commented Oct 28, 2018

I‘d like to see this change applied; GetComputerName returns the truncated netbios name which is not necessarily the same as the system received from DHCP for DNS.

I’d use ComputerNamePhysicalDnsHostname though because it probably does less DNS calls, and perhaps also GetComputerNameExW instead of *A because DNS can contain Unicode probably.

At least that’s what Python does in socket.gethostname and I believe Python receive a good deal of real world testing here.

@fengcen
Copy link
Copy Markdown
Contributor

fengcen commented Jan 19, 2019

Sorry guys. I have no time and interest in Rust right now. This project is going to Abandoned status.

@Jake-Shadle
Copy link
Copy Markdown
Author

@fengcen in that case I wouldn't mind taking over ownership of this crate since I work in rust full time now. 🙂

@swsnr
Copy link
Copy Markdown

swsnr commented Feb 5, 2019

@Jake-Shadle I did gethostname.rs meanwhile.

@Jake-Shadle
Copy link
Copy Markdown
Author

Cool, guess we can just use that instead, thanks!

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