[PATCH] D114342: ConvertUTF, new wrapper API

[Michael Spencer via Phabricator via llvm-commits]

Bigcheese requested changes to this revision.
Bigcheese added a comment.
This revision now requires changes to proceed.

Once these issues are fixed this should be fine.



================
Comment at: llvm/lib/Support/ConvertUTFWrapper.cpp:148
+  // Avoid OOB by returning early on empty input.
+  if (SrcBytes.empty())
+    return true;
----------------
I believe this actually needs an additional check that that `SrcBytes.size()` is `< 4` and return false.


================
Comment at: llvm/lib/Support/ConvertUTFWrapper.cpp:172
+  // enough that we can fit a null terminator without reallocating.
+  Out.resize(SrcBytes.size() + 1);
+  UTF8 *Dst = reinterpret_cast<UTF8 *>(&Out[0]);
----------------
This is technically correct, but it's implicit in that the max number of UTF8 code units per code point is the same as `sizeof(UTF32)`. Would be nice to have a comment.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114342/new/

https://reviews.llvm.org/D114342