[PATCH] D67867: [libc] Add few docs and implementation of strcpy and strcat.

David Chisnall via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 27 03:13:09 PDT 2019


theraven added inline comments.


================
Comment at: libc/include/math.h:20
+
+long double acosl(long double);
+
----------------
The `long long` functions should be exposed only for C99 or later and a version of C++ that supports the `long long` type.  


================
Comment at: libc/include/string.h:12
+
+#include <stddef.h> // For size_t
+
----------------
Namespace pollution.  The standard expects `size_t` to be exposed by this header, but not the other types in `stddef.h`.  Software that relies on this pollution is non-portable (and will break on existing libc implementations that follow the standard).


================
Comment at: libc/include/string.h:18
+
+void *memcpy(void *, const void *, size_t);
+
----------------
Missing `restrict` qualifiers on this and many other standard functions in this file.  The standard defines `memcpy` as:

```
void *memcpy(void * restrict s1,
const void * restrict s2,
size_t n);
```

I'm surprised that clang doesn't warn about the declaration of `memcpy` with incorrect types - it usually notices missing qualifiers.


================
Comment at: libc/src/string/strcpy/strcpy_test.cpp:16
+  std::string abc = "abc";
+  char *dest = new char[4];
+
----------------
Having the libc test suite depend on a working C++ runtime and standard library is likely to make unit testing the library difficult.  The implementation of `std::string` almost certainly depends on several `libc` functions and `new` and `delete` probably do as well.  This means that we can test the namespaced libc functions in an environment that already has a libc, but we can't easily extend these tests to run in a process (or other environment) that has only LLVM libc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67867





More information about the llvm-commits mailing list