> On Dec. 20, 2016, 12:29 a.m., Daniel Pravat wrote: > > 3rdparty/stout/include/stout/windows/os.hpp, line 748 > > <https://reviews.apache.org/r/54877/diff/1/?file=1589098#file1589098line748> > > > > I don't think the conversion to UTF-8 is appropiate here. > > Andrew Schwartzmeyer wrote: > What would you convert it to? It's currently in UTF-16, and Windows paths > are allowed to have (almost) any Unicode character. > > Alex Clemmer wrote: > I think he's saying that bad things will happen if you use strings that > have Unicode in them, so it's probably better to just error out. > > Andrew Schwartzmeyer wrote: > I'm not following... That is a case our codebase must deal with, NTFS > paths are stored in Unicode: > > > https://msdn.microsoft.com/en-us/library/windows/desktop/dd317748(v=vs.85).aspx > > and on top of that, Windows file paths can: > > > Use any character in the current code page for a name, including > Unicode characters and characters in the extended character set (128–255) > > > https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx > > So what I'm saying is that bad things _must not happen_ if we have paths > with non-ASCII characters in them; we need to handle that at any point we > deal with file paths on Windows. > > I'm not saying this function is likely to return a path like `C:\ÂÑÐ¥`, > but that would be a valid file path, and so we must be treating all our paths > on Windows as Unicode. > > If this is not the case, are purposefully constraining ourselves to > ASCII, and if so, why?
Sorry, I meant bad things will happen to Mesos if you give it strings with unicode in them. Mesos itself does not deal with Unicode gracefully. - Alex ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54877/#review159672 ----------------------------------------------------------- On Dec. 19, 2016, 11:20 p.m., Andrew Schwartzmeyer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54877/ > ----------------------------------------------------------- > > (Updated Dec. 19, 2016, 11:20 p.m.) > > > Review request for mesos, Daniel Pravat, Alex Clemmer, and Joseph Wu. > > > Repository: mesos > > > Description > ------- > > The API `SHGetKnownFolderPath` requires `Shell32.dll`, > which is not available on Nano server. > The equivalent API `GetAllUsersProfileDirectory` > only requires `Userenv.dll`, which is available on Nano. > > This API is also friendlier, as we own the allocation. > > The Unicode version `GetAllUsersProfileDirectoryW` is > explicitly used so that we are guaranteed a Unicode path, > which we then convert from UTF-16 to UTF-8, > instead of using the ASCII version which depends on a > varying Windows code-page, and is not recommended. > > A `vector<wchar_t>` is used over a `wstring` to avoid dealing > with the placement of the null-terminating character. > > > Diffs > ----- > > 3rdparty/stout/include/stout/windows.hpp > e641c46d033372e1b6c9f9c066b1ad4957d55088 > 3rdparty/stout/include/stout/windows/os.hpp > 5cd92545a49648e39e8eb7cf131895e9cfc97902 > > Diff: https://reviews.apache.org/r/54877/diff/ > > > Testing > ------- > > cmake && msbuild, attach agent to master and check default `runtime_dir` > value. > > > Thanks, > > Andrew Schwartzmeyer > >