[libc-commits] [PATCH] D91399: Add strncpy implementation.

[Siva Chandra via Phabricator via libc-commits]

sivachandra added a comment.

Thanks for the patch. I have few nits and a question inline.



================
Comment at: libc/src/string/strncpy.cpp:19
+                                    const char *__restrict src, size_t n) {
+  for (size_t i = 0, j = 0; i < n; ++i) {
+    if (*(src + j) != '\0') {
----------------
WDYT about a `memcpy` followed by a `memset` (for the null characters)? Both these functions in libcs are //optimized// in some manner so we will be leveraging that. We will need an additional `strlen` operation though.


================
Comment at: libc/test/src/string/strncpy_test.cpp:1
+//===-- Unittests for strncpy----------------------------------------------===//
+//
----------------
Nit: Fix this. You probably need a space after `strncpy`.


================
Comment at: libc/test/src/string/strncpy_test.cpp:12
+
+TEST(StrNCpyTest, ReturnRightValue) {
+  const char src[] = {'a', 'b', 'c', '\0'};
----------------
Nit: May be name it `SameSrcSizeAndCopySize` instead of `ReturnRightValue`?


================
Comment at: libc/test/src/string/strncpy_test.cpp:20
+
+TEST(StrNCpyTest, OffsetDest) {
+  const char src[] =      {'a', 'b', '\0'};
----------------
Nit: May be name it `CopySizeNotEqualToSrcSize` instead of `OffsetDest`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91399