[libcxx-commits] [PATCH] D98207: [SystemZ][z/OS] Missing wchar functions libc++

[Zibi Sarbino via Phabricator via libcxx-commits]

zibi added a comment.

Sorry for the nit picking and I know this is coming from existing Windows implementation but we have the opportunity to clean it up.
Most comments refer to both functions.



================
Comment at: libcxx/src/support/ibm/mbsnrtowcs.cpp:1
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
----------------
Can you replace first 2 lines with 

//===----------------------- mbsnrtowcs.cpp -------------------------------===//

This will make consistent with majority of other source files.


================
Comment at: libcxx/src/support/ibm/mbsnrtowcs.cpp:17
+// null terminator. When dst is NULL, no characters are copied
+// and no "out" parameters are updated.
+// Returns (size_t) -1: an incomplete sequence encountered.
----------------
Can you clean up this description so it's easer to read? 

For example:
`and no "out" parameters are updated`.  Does it refer to `dst` or `src`?  Also if you refer to the parameter name please annotate it to make it stand out.



================
Comment at: libcxx/src/support/ibm/mbsnrtowcs.cpp:26
+  const size_t terminated_sequence = static_cast<size_t>(0);
+  //const size_t invalid_sequence = static_cast<size_t>(-1);
+  const size_t incomplete_sequence = static_cast<size_t>(-2);
----------------
Any reason to keep this commented out line around?


================
Comment at: libcxx/src/support/ibm/mbsnrtowcs.cpp:44
+      // Converts one multi byte character.
+      // if result > 0, it's the size in bytes of that character.
+      // othewise if result is zero it indicates the null character has been found.
----------------
Apparently comments need to be proper sentences. Also fix the comment so it actually refer to `char_size` and not result.

It looks like result will never be set to >0 value..


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

https://reviews.llvm.org/D98207