Fixes incorrect wide string conversion on win32

[JessyDL]

Fixes issue #1515 that I reported.

The PR this fixes incorrectly tried to convert an existing wide string (GetComputerName returns a wide string when building with UNICODE) into another wide string (std::vector<wchar_t> converted specifically), and then tried to simply initialize an std::string with a wchar_t*.

I preserved the error handling from the original code (ignoring the error and just returning nothing). Depending on your affinity to exceptions, that PR originally replaced std::wstring_convert which would throw an std::range_error. I'm happy to preserve that behaviour.

I locally ran this to verify the output, feel free to test this more rigorously.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[dmah42]

i think this check is redundant if you don't capture the returned length from the second call to WideCharToMultiByte

[JessyDL]

Yeah I agree the check is redundant. Even with them functioning they still would be redundant as the documentation specifies 0 to be returned when the WideCharToMultiByte fails so it's safe to remove them both. I kept them around for consistency reasons with the previous PR.

I've resolved this by removing both error checks, as the downstream code would be safe to execute regardless.

[dmah42]

i can't see how this causes the issues in the asan checks, given the change is windows-only. but if you could just either check the return value of the second call or remove the redundant check, then we can get this merged :)

[JessyDL]

Yeah, very strange asan would trigger on that, I'll need to check it out

[JessyDL]

Yeah, honestly no clue why ASAN would trigger for code it shouldn't be concerned with to begin with. I can remove the len var entirely, but that would be at the detriment of the code readability.

The second call to WideCharToMultiByte's return is superfluous already, so checking it again would make no sense.

[JessyDL]

I'm reluctant to merge this PR with ASAN failing. I don't really have the time to debug why that's the case. I'll be leaving this open, and hopefully someone else can come along and figure out why these Windows-only lines trip ASAN on Ubuntu.

[dmah42]

i'm poking at it today.

[dmah42]

i have convinced myself that this is an issue with the standard library rather than an issue with benchmark.

in this case, we are passing in a string from the command line that is used as a regex to filter the benchmarks. asan is finding an issue where std::regex creates a std::runtime_error but deallocates a std::range_error. both of these in libc++.

nothing here is related to your change so i'm happy to merge.

[JessyDL]

Thanks for checking it out! That sounds exceptionally weird (heh).