[libc-commits] [PATCH] D74021: Created uChar implementation for libc

[Alex Brachet via Phabricator via libc-commits]

abrachet added a comment.

For reference this is musl's implementation of c16rtomb http://git.musl-libc.org/cgit/musl/tree/src/multibyte/c16rtomb.c.

> As for tests, I'm not sure where to begin with that, can anyone point me in the right direction?

You could replicate the example that cppreference has. https://en.cppreference.com/w/c/string/multibyte/c16rtomb. Otherwise where have you seen these functions use in the wild? I have never seen it so I can't give any real world examples unfortunately.

libcxx has pretty sparse tests https://github.com/llvm/llvm-project/tree/master/libcxx/test/std/strings https://github.com/llvm/llvm-project/tree/master/libcxx/test/libcxx/strings I'm afraid.

You could look at the standard library of more modern languages. For example Rust made a big deal about Utf16 strings, you could hunt down those tests https://doc.rust-lang.org/std/primitive.str.html.

And in musl's tests (cloned from git://repo.or.cz/libc-test) they have libc-test/src/functional/clocale_mbfuncs.c, which is the closest I could find.



================
Comment at: libc/src/uchar/c16rtomb.cpp:25
+    StringSize = 1;
+    s = new char(StringSize);
+    s[0] = c16 & 0x7F;
----------------
For what its worth, this allocates one char and initializes it to StringSize. If you wanted to allocate StringSize chars you need `new char[StringSize]`. But `new` isn't safe to use for us, and I think it wouldn't even link because we have `-nostdlib` specified.


```lang=C
void changeParam(int a) {
    a = 5;
}

int main() {
   int a = 0;
   changeParam(a);
   assert(!a); // passes, a not changed.
}
```
The same is true with pointers if I pass my `char *` to c16rtomb, if you reassign to it by malloc or new, it changes the local variable but not the callers variable. These functions (unless passed a nullptr) assume that `s` is already pointing to enough valid memory.


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

https://reviews.llvm.org/D74021