[libc] Add strncpy implementation.

[cheng.w]

Summary

Add libc strncpy implementation.

[sivachandra]

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

libc/src/string/strncpy.cpp

20

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.

libc/test/src/string/strncpy_test.cpp

2

Nit: Fix this. You probably need a space after strncpy.

13

Nit: May be name it SameSrcSizeAndCopySize instead of ReturnRightValue?

21

Nit: May be name it CopySizeNotEqualToSrcSize instead of OffsetDest.

[cheng.w]

Thanks Siva.

libc/src/string/strncpy.cpp

20

By referring to strcpy, I used memcpy and strlen at first.
But it seems that this method avoids the overhead of:

• strlen goes through src for the length.
• function calls.

I am not sure whick method is more appropriate. So I upload it for suggestions.

[sivachandra]

Thanks for your patience.

libc/src/string/strncpy.cpp

20

By referring to strcpy, I used memcpy and strlen at first.
But it seems that this method avoids the overhead of:

• strlen goes through src for the length.
• function calls.

I am not sure whick method is more appropriate. So I upload it for suggestions.

I am not opposed to any approach. At the same time, I do not feel qualified enough to drive an opinion. So, adding @gchatelet for guidance.

[gchatelet]

Thx for the patch!

libc/src/string/strncpy.cpp

20

It's hard to be assertive on which one is best here. It certainly depends on how strncpy is used (buffer sizes, value of n, size of the zeroing, ...).
I'd say let's keep the implementation simple and optimize if we see performance issues.

23

There are two modes for computing strncpy:

1. copy up until \0 is found
2. fill with \0

You can only ever go from 1 to 2, never the opposite.
j is only used in 1 and is incremented at each loop iteration.

So it seems to me that you don't need j, you can just use i.

I've been fiddling a bit with different implementations (https://godbolt.org/z/Eb9xq9) and the following one seems simple and nice

char *strncpy(char *__restrict dest, const char *__restrict src, size_t n) {
  size_t i = 0;
  for (; i < n && src[i] != '\0'; ++i) dest[i] = src[i];
  for (; i < n; ++i) dest[i] = '\0';
  return dest;
}

Added bonus, the compiler recognizes for (; i < n; ++i) dest[i] = '\0'; as a memset which lowers this function's footprint. IMHO the filling with zero part is not the most used part of the algorithm so it's nice to outline it.

25

Can you use dest[i] instead of *(dest + i)? I believe it's more readable.
Here and above (same for src).

libc/test/src/string/strncpy_test.cpp

45

Would you mind splitting this into 3 tests to ease readability?
Also a test fixture like so would improve readability.

struct StrncpyFixture : public __llvm_libc::testing::Test {
  void check_strncpy(__llvm_libc::cpp::MutableArrayRef<char> dst,
                     __llvm_libc::cpp::ArrayRef<char> src, size_t n,
                     __llvm_libc::cpp::ArrayRef<char> expected) {
    // Making sure we don't overflow buffer.
    ASSERT_GE(dst.size(), n);
    // Making sure strncpy returns dst.
    ASSERT_EQ(__llvm_libc::strncpy(dst.data(), src.data(), n), dst.data());
    // Expected must be of the same size as Dest.
    ASSERT_EQ(dst.size(), expected.size());
    for (size_t i = 0; i < expected.size(); ++i)
      ASSERT_EQ(expected[i], dst[i]);
  }
};

TEST_F(StrncpyFixture, Untouched) {
  char dst[] = {'a', 'b'};
  char src[] = {'x', '\0'};
  char expected[] = {'a', 'b'};
  check_strncpy(dst, src, 0, expected);
}

TEST_F(StrncpyFixture, CopyOne) {
  char dst[] = {'a', 'b'};
  char src[] = {'x', 'y'};
  char expected[] = {'x', 'b'}; // no \0 is appended
  check_strncpy(dst, src, 1, expected);
}

TEST_F(StrncpyFixture, CopyNull) {
  char dst[] = {'a', 'b'};
  char src[] = {'\0', 'y'};
  char expected[] = {'\0', 'b'};
  check_strncpy(dst, src, 1, expected);
}

TEST_F(StrncpyFixture, CopyPastSrc) {
  char dst[] = {'a', 'b'};
  char src[] = {'\0', 'y'};
  char expected[] = {'\0', '\0'};
  check_strncpy(dst, src, 2, expected);
}

[cheng.w]

Take code from Chatelet. Fix some nits.

[cheng.w]

I am sorry to occupy too much time of you both. I initially just want to optimize it a little bit.

[gchatelet]

Thank you for your contribution!
Please wait for @sivachandra before submitting.

libc/config/linux/aarch64/entrypoints.txt

set(TARGET_LIBC_ENTRYPOINTS
     libc.src.string.memcpy
     libc.src.string.memset
     libc.src.string.memrchr
     libc.src.string.strcat
     libc.src.string.strchr
     libc.src.string.strcpy
     libc.src.string.strcspn
     libc.src.string.strlen
+    libc.src.string.strncpy
     libc.src.string.strnlen
     libc.src.string.strpbrk
     libc.src.string.strrchr
     libc.src.string.strspn
     libc.src.string.strstr
     libc.src.string.strtok
     libc.src.string.strtok_r
 )

libc/config/linux/x86_64/entrypoints.txt

set(TARGET_LIBC_ENTRYPOINTS
     libc.src.string.memrchr
     libc.src.string.memset
     libc.src.string.strcat
     libc.src.string.strchr
     libc.src.string.strcmp
     libc.src.string.strcpy
     libc.src.string.strcspn
     libc.src.string.strlen
+    libc.src.string.strncpy
     libc.src.string.strnlen
     libc.src.string.strpbrk
     libc.src.string.strrchr
     libc.src.string.strspn
     libc.src.string.strstr
     libc.src.string.strtok
     libc.src.string.strtok_r
 

libc/src/string/CMakeLists.txt

add_entrypoint_object(
   strstr
   SRCS
     strstr.cpp
   HDRS
     strstr.h
 )
 
 add_entrypoint_object(
+  strncpy
+  SRCS
+    strncpy.cpp
+  HDRS
+    strncpy.h
+)
+
+add_entrypoint_object(
   strnlen
   SRCS
     strnlen.cpp
   HDRS
     strnlen.h
   DEPENDS
     .string_utils
 )

libc/src/string/strncpy.h

+//===-- Implementation header for strncpy -----------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_STRING_STRNCPY_H
+#define LLVM_LIBC_SRC_STRING_STRNCPY_H
+
+#include <stddef.h>
+
+namespace __llvm_libc {
+
+char *strncpy(char *__restrict dest, const char *__restrict src, size_t n);
+
+} // namespace __llvm_libc
+
+#endif // LLVM_LIBC_SRC_STRING_STRNCPY_H

libc/src/string/strncpy.cpp

+//===-- Implementation of strncpy -----------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/string/strncpy.h"
+
+#include "src/__support/common.h"
+#include <stddef.h> // For size_t.
+
+namespace __llvm_libc {
+
+char *LLVM_LIBC_ENTRYPOINT(strncpy)(char *__restrict dest,
+                                    const char *__restrict src, size_t n) {
+  size_t i = 0;
+  // Copy up until \0 is found.
+  for (; i < n && src[i] != '\0'; ++i)
+    dest[i] = src[i];
+  // When n>strlen(src), n-strlen(src) \0 are appended.
+  for (; i < n; ++i)
+    dest[i] = '\0';
+  return dest;
+}
+
+} // namespace __llvm_libc

libc/test/src/string/CMakeLists.txt

SUITE
     libc_string_unittests
   SRCS
     strstr_test.cpp
   DEPENDS
     libc.src.string.strstr
 )
 
 add_libc_unittest(
+  strncpy_test
+  SUITE
+    libc_string_unittests
+  SRCS
+    strncpy_test.cpp
+  DEPENDS
+    libc.src.string.strncpy
+)
+
+add_libc_unittest(
   strnlen_test
   SUITE
     libc_string_unittests
   SRCS
     strnlen_test.cpp
   DEPENDS
     libc.src.string.strnlen
 )

libc/test/src/string/strncpy_test.cpp

+//===-- Unittests for strncpy ---------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/string/strncpy.h"
+#include "utils/CPP/ArrayRef.h"
+#include "utils/UnitTest/Test.h"
+#include <stddef.h> // For size_t.
+
+class StrncpyTest : public __llvm_libc::testing::Test {
+public:
+  void check_strncpy(__llvm_libc::cpp::MutableArrayRef<char> dst,
+                     const __llvm_libc::cpp::ArrayRef<char> src, size_t n,
+                     const __llvm_libc::cpp::ArrayRef<char> expected) {
+    // Making sure we don't overflow buffer.
+    ASSERT_GE(dst.size(), n);
+    // Making sure strncpy returns dst.
+    ASSERT_EQ(__llvm_libc::strncpy(dst.data(), src.data(), n), dst.data());
+    // Expected must be of the same size as dst.
+    ASSERT_EQ(dst.size(), expected.size());
+    // Expected and dst are the same.
+    for (size_t i = 0; i < expected.size(); ++i)
+      ASSERT_EQ(expected[i], dst[i]);
+  }
+};
+
+TEST_F(StrncpyTest, Untouched) {
+  char dst[] = {'a', 'b'};
+  const char src[] = {'x', '\0'};
+  const char expected[] = {'a', 'b'};
+  check_strncpy(dst, src, 0, expected);
+}
+
+TEST_F(StrncpyTest, CopyOne) {
+  char dst[] = {'a', 'b'};
+  const char src[] = {'x', 'y'};
+  const char expected[] = {'x', 'b'}; // no \0 is appended
+  check_strncpy(dst, src, 1, expected);
+}
+
+TEST_F(StrncpyTest, CopyNull) {
+  char dst[] = {'a', 'b'};
+  const char src[] = {'\0', 'y'};
+  const char expected[] = {'\0', 'b'};
+  check_strncpy(dst, src, 1, expected);
+}
+
+TEST_F(StrncpyTest, CopyPastSrc) {
+  char dst[] = {'a', 'b'};
+  const char src[] = {'\0', 'y'};
+  const char expected[] = {'\0', '\0'};
+  check_strncpy(dst, src, 2, expected);
+}