[PATCH] Add getenv() wrapper that works on multibyte environment variable.

Reid Kleckner rnk at google.com
Mon Sep 9 15:15:42 PDT 2013



================
Comment at: lib/Support/ConvertUTFWrapper.cpp:95
@@ +94,3 @@
+  // Just allocate enough space up front. We'll shrink it later.
+  Out.resize(SrcBytes.size() * 2);
+  UTF16 *Dst = reinterpret_cast<UTF16 *>(&Out[0]);
----------------
Is that going to be enough?  UTF-16 has variable width codepoints too right?

================
Comment at: include/llvm/Support/ConvertUTF.h:240
@@ +239,3 @@
+ */
+bool convertUTF8ToUTF16String(ArrayRef<char> SrcBytes, std::string &Out);
+
----------------
I don't think we should use std::strings of UTF16 across any function call boundary.  What can we do instead?  I guess wchar_t is a different size on different platforms, so std::wstring is right out.  Does std::basic_string<UTF16> work?

================
Comment at: lib/Support/Environ.cpp:26
@@ +25,3 @@
+
+/// Returnsthe environment variable \arg Name's value as a string encoded in
+/// UTF-8. \arg Name is assumed to be in UTF-8 encoding.
----------------
"Returnsthe"

================
Comment at: lib/Support/Environ.cpp:65
@@ +64,3 @@
+
+Optional<std::string> sys::env::getenv(StringRef Name) {
+  const char *Val = ::getenv(Name.str().c_str());
----------------
I'd put this one first, because it's simple.

================
Comment at: include/llvm/Support/ConvertUTF.h:240
@@ +239,3 @@
+ */
+bool convertUTF8ToUTF16String(ArrayRef<char> SrcBytes, std::string &Out);
+
----------------
Reid Kleckner wrote:
> I don't think we should use std::strings of UTF16 across any function call boundary.  What can we do instead?  I guess wchar_t is a different size on different platforms, so std::wstring is right out.  Does std::basic_string<UTF16> work?
Do you think this helper is generally useful?  Can we get away with calling MultiByteToWideChar(CP_UTF8, ...) instead, since we only need this functionality in Windows code?


http://llvm-reviews.chandlerc.com/D1612



More information about the llvm-commits mailing list