Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions libc/config/linux/x86_64/entrypoints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,7 @@ set(TARGET_LIBC_ENTRYPOINTS
libc.src.wchar.btowc
libc.src.wchar.wcslen
libc.src.wchar.wctob
libc.src.wchar.wmemset

# sys/uio.h entrypoints
libc.src.sys.uio.writev
Expand Down
8 changes: 8 additions & 0 deletions libc/include/wchar.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,11 @@ functions:
return_type: wint_t
arguments:
- type: int
- name: wmemset
standards:
- stdc
return_type: wchar_t*
arguments:
- type: wchar_t*
- type: wchar_t
- type: size_t
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: fix missing newline

12 changes: 12 additions & 0 deletions libc/src/wchar/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,15 @@ add_entrypoint_object(
libc.hdr.wchar_macros
libc.src.__support.wctype_utils
)

add_entrypoint_object(
wmemset
SRCS
wmemset.cpp
HDRS
wmemset.h
DEPENDS
libc.hdr.types.size_t
libc.hdr.types.wchar_t
libc.src.__support.wctype_utils
)
24 changes: 24 additions & 0 deletions libc/src/wchar/wmemset.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
//===-- Implementation of wmemset -----------------------------------------===//
//
// 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/wchar/wmemset.h"

#include "hdr/types/size_t.h"
#include "hdr/types/wchar_t.h"
#include "src/__support/common.h"

namespace LIBC_NAMESPACE_DECL {

LLVM_LIBC_FUNCTION(wchar_t *, wmemset, (wchar_t * s, wchar_t c, size_t n)) {
for (size_t i = 0; i < n; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The LLVM coding standards recommend skipping the braces for one line for loops: https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

s[i] = c;
}
return s;
}

} // namespace LIBC_NAMESPACE_DECL
23 changes: 23 additions & 0 deletions libc/src/wchar/wmemset.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
//===-- Implementation header for wmemset
//----------------------------------===//
//
// 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_WCHAR_WMEMSET_H
#define LLVM_LIBC_SRC_WCHAR_WMEMSET_H

#include "hdr/types/size_t.h"
#include "hdr/types/wchar_t.h"
#include "src/__support/macros/config.h"

namespace LIBC_NAMESPACE_DECL {

wchar_t *wmemset(wchar_t *s, wchar_t c, size_t n);

} // namespace LIBC_NAMESPACE_DECL

#endif // LLVM_LIBC_SRC_WCHAR_WMEMSET_H
12 changes: 12 additions & 0 deletions libc/test/src/wchar/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,15 @@ add_libc_test(
DEPENDS
libc.src.wchar.wctob
)

add_libc_test(
wmemset_test
SUITE
libc_wchar_unittests
SRCS
wmemset_test.cpp
DEPENDS
libc.hdr.types.size_t
libc.hdr.types.wchar_t
libc.src.wchar.wmemset
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: fix missing newline

87 changes: 87 additions & 0 deletions libc/test/src/wchar/wmemset_test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
//===-- Unittests for wmemset ---------------------------------------------===//
//
// 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 "hdr/types/size_t.h"
#include "hdr/types/wchar_t.h"
#include "src/wchar/wmemset.h"
#include "test/UnitTest/Test.h"

TEST(LlvmLibcWMemsetTest, SmallStringBoundCheck) {
wchar_t *str = new wchar_t[5];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use new here that puts the data on the heap. When possible, it's preferred to put test variables on the stack to simplify memory management. To do that you define the variable as a local array instead of just a pointer.

Same recommendation on all these tests, but for simplicity I'm just marking this one.

Suggested change
wchar_t *str = new wchar_t[5];
wchar_t str[5];

for (int i = 0; i < 5; i++) {
str[i] = 'A';
}

wchar_t *output = LIBC_NAMESPACE::wmemset(str + 1, 'B', 3);

EXPECT_EQ(output, str + 1);

EXPECT_TRUE(str[0] == (wchar_t)'A');
EXPECT_TRUE(str[1] == (wchar_t)'B');
EXPECT_TRUE(str[2] == (wchar_t)'B');
EXPECT_TRUE(str[3] == (wchar_t)'B');
EXPECT_TRUE(str[4] == (wchar_t)'A');
}

TEST(LlvmLibcWMemsetTest, LargeStringBoundCheck) {
const int str_size = 1000;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: make this constexpr

Suggested change
const int str_size = 1000;
constexpr int STR_SIZE = 1000;

wchar_t *str = new wchar_t[str_size];
for (int i = 0; i < str_size; i++) {
str[i] = 'A';
}

wchar_t *output = LIBC_NAMESPACE::wmemset(str + 1, 'B', str_size - 2);

EXPECT_EQ(output, str + 1);

EXPECT_TRUE(str[0] == (wchar_t)'A');
for (int i = 1; i < str_size - 1; i++) {
EXPECT_TRUE(str[i] == (wchar_t)'B');
}
EXPECT_TRUE(str[str_size - 1] == (wchar_t)'A');
}

TEST(LlvmLibcWMemsetTest, WChar_Size_Small) {
// ensure we can handle 32 bit values
wchar_t *str = new wchar_t[5];
const wchar_t magic = INT32_MAX;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not necessarily safe to assume wchar_t is 32 bits. Instead it's better to use the provided WCHAR_MAX macro.

It would also be good to give magic a more descriptive name.

Suggested change
const wchar_t magic = INT32_MAX;
const wchar_t magic = WCHAR_MAX;


for (int i = 0; i < 5; i++) {
str[i] = 'A';
}

wchar_t *output = LIBC_NAMESPACE::wmemset(str + 1, magic, 3);

EXPECT_EQ(output, str + 1);

EXPECT_TRUE(str[0] == (wchar_t)'A');
EXPECT_TRUE(str[1] == magic);
EXPECT_TRUE(str[2] == magic);
EXPECT_TRUE(str[3] == magic);
EXPECT_TRUE(str[4] == (wchar_t)'A');
}

TEST(LlvmLibcWMemsetTest, WChar_Size_Large) {
// ensure we can handle 32 bit values
const int str_size = 1000;
const wchar_t magic = INT32_MAX;
wchar_t *str = new wchar_t[str_size];
for (int i = 0; i < str_size; i++) {
str[i] = 'A';
}

wchar_t *output = LIBC_NAMESPACE::wmemset(str + 1, magic, str_size - 2);

EXPECT_EQ(output, str + 1);

EXPECT_TRUE(str[0] == (wchar_t)'A');
for (int i = 1; i < str_size - 1; i++) {
EXPECT_TRUE(str[i] == magic);
}
EXPECT_TRUE(str[str_size - 1] == (wchar_t)'A');
}