This is an archive of the discontinued LLVM Phabricator instance.

[profile] use GetComputerNameExW instead of gethostname on Windows
ClosedPublic

Authored by inglorion on Nov 28 2016, 4:10 PM.

Details

Summary

In profile data paths, we replace "%h" with the hostname of the machine the program is running on. On Windows, we used gethostname() to obtain the hostname. This requires linking with ws2_32. With this change, we instead get the hostname from GetComputerNameExW(), which does not require ws2_32.

Event Timeline

inglorion updated this revision to Diff 79474.Nov 28 2016, 4:10 PM
inglorion retitled this revision from to [profile] use GetComputerNameEx instead of gethostname on Windows.
inglorion updated this object.
inglorion added reviewers: amccarth, rnk, vsk.
inglorion added subscribers: hans, ruiu, zturner.

This was suggested by @amccarth on D27086.

rnk added inline comments.Nov 28 2016, 4:12 PM
lib/profile/InstrProfilingPort.h
18

This code seems to go out of its way to not include windows.h, I'd like to preserve that. Can we sink the implementation of compiler_rt_gethostname to a .c file?

inglorion added inline comments.Nov 28 2016, 4:23 PM
lib/profile/InstrProfilingPort.h
18

Good point, will do.

inglorion updated this revision to Diff 79477.Nov 28 2016, 4:24 PM

Moved the implementation of the compatibility function to a .c file, as requested by @rnk.

amccarth edited edge metadata.Nov 28 2016, 4:36 PM

I like where this is going.

lib/profile/InstrProfilingUtil.c
71 ↗(On Diff #79477)

TCHAR?! Since we're calling WideCharToMultiByte, it seems we're assuming "Unicode" mode, so I'd write WCHAR and keep it explicit.

82 ↗(On Diff #79477)

This doesn't terminate Name. There's a gotcha in this API that if you provide the length for the input string (BufferSize here), it'll process exactly that many "characters" and not terminate the output string.

See the description of cchWideChar in https://msdn.microsoft.com/en-us/library/windows/desktop/dd374130(v=vs.85).aspx

If you know that Buffer is terminated, then it's safest to use -1 for cchWideChar.

inglorion updated this revision to Diff 79480.Nov 28 2016, 4:47 PM
inglorion edited edge metadata.

addressed problems pointed out by @amccarth

amccarth accepted this revision.Nov 28 2016, 4:55 PM
amccarth edited edge metadata.

I'm happy. Thanks for fixing those warning messages, too.

This revision is now accepted and ready to land.Nov 28 2016, 4:55 PM
zturner added inline comments.Nov 28 2016, 5:32 PM
lib/profile/InstrProfilingUtil.c
71 ↗(On Diff #79477)

If we're assuming unicode mode, then let's call GetComputeNameExW.

inglorion updated this revision to Diff 79501.Nov 28 2016, 6:54 PM
inglorion retitled this revision from [profile] use GetComputerNameEx instead of gethostname on Windows to [profile] use GetComputerNameExW instead of gethostname on Windows.
inglorion updated this object.
inglorion edited edge metadata.

Changed to use GetComputerNameExW. Thanks for all the suggestions!

inglorion updated this revision to Diff 79569.Nov 29 2016, 7:24 AM

clang-format

This revision was automatically updated to reflect the committed changes.