Skip to content

Commit 2e83f10

Browse files
michaelrj-googlemichaelrj-google
authored
[libc] Cleanup mman functions and tests (#159657)
Remove functions depending on each other, cleanup internal errno in shm_common, remove various unnecessary includes.
1 parent 55f230f commit 2e83f10

File tree

9 files changed

+55
-55
lines changed

9 files changed

+55
-55
lines changed

‎libc/src/sys/mman/linux/CMakeLists.txt‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -184,11 +184,10 @@ add_header_library(
184184
HDRS
185185
shm_common.h
186186
DEPENDS
187+
libc.hdr.errno_macros
187188
libc.src.__support.CPP.array
188189
libc.src.__support.CPP.string_view
189-
libc.src.__support.CPP.optional
190-
libc.src.__support.common
191-
libc.src.errno.errno
190+
libc.src.__support.error_or
192191
libc.src.string.memory_utils.inline_memcpy
193192
)
194193

@@ -199,8 +198,8 @@ add_entrypoint_object(
199198
HDRS
200199
../shm_open.h
201200
DEPENDS
202-
libc.src.fcntl.open
203201
libc.hdr.types.mode_t
202+
libc.src.errno.errno
204203
.shm_common
205204
)
206205

@@ -211,6 +210,6 @@ add_entrypoint_object(
211210
HDRS
212211
../shm_unlink.h
213212
DEPENDS
214-
libc.src.unistd.unlink
213+
libc.src.errno.errno
215214
.shm_common
216215
)

‎libc/src/sys/mman/linux/shm_common.h‎

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,13 @@
66
//
77
//===----------------------------------------------------------------------===//
88

9+
#include "hdr/errno_macros.h"
910
#include "src/__support/CPP/array.h"
10-
#include "src/__support/CPP/optional.h"
1111
#include "src/__support/CPP/string_view.h"
12-
#include "src/__support/libc_errno.h"
12+
#include "src/__support/error_or.h"
1313
#include "src/__support/macros/config.h"
1414
#include "src/string/memory_utils/inline_memcpy.h"
1515

16-
// TODO: clean this up.
17-
// 1. Change from optional to ErrorOr, and return the errno instead of setting
18-
// it here.
19-
// 2. Replace inline memcpy with __builtin_memcpy
20-
2116
// TODO: Get PATH_MAX via https://github.com/llvm/llvm-project/issues/85121
2217
#include <linux/limits.h>
2318

@@ -28,24 +23,18 @@ namespace shm_common {
2823
LIBC_INLINE_VAR constexpr cpp::string_view SHM_PREFIX = "/dev/shm/";
2924
using SHMPath = cpp::array<char, NAME_MAX + SHM_PREFIX.size() + 1>;
3025

31-
LIBC_INLINE cpp::optional<SHMPath> translate_name(cpp::string_view name) {
26+
LIBC_INLINE ErrorOr<SHMPath> translate_name(cpp::string_view name) {
3227
// trim leading slashes
3328
size_t offset = name.find_first_not_of('/');
34-
if (offset == cpp::string_view::npos) {
35-
libc_errno = EINVAL;
36-
return cpp::nullopt;
37-
}
29+
if (offset == cpp::string_view::npos)
30+
return Error(EINVAL);
3831
name = name.substr(offset);
3932

4033
// check the name
41-
if (name.size() > NAME_MAX) {
42-
libc_errno = ENAMETOOLONG;
43-
return cpp::nullopt;
44-
}
45-
if (name == "." || name == ".." || name.contains('/')) {
46-
libc_errno = EINVAL;
47-
return cpp::nullopt;
48-
}
34+
if (name.size() > NAME_MAX)
35+
return Error(ENAMETOOLONG);
36+
if (name == "." || name == ".." || name.contains('/'))
37+
return Error(EINVAL);
4938

5039
// prepend the prefix
5140
SHMPath buffer;

‎libc/src/sys/mman/linux/shm_open.cpp‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "src/sys/mman/shm_open.h"
10+
1011
#include "hdr/fcntl_macros.h"
1112
#include "hdr/types/mode_t.h"
1213
#include "src/__support/OSUtil/fcntl.h"
14+
#include "src/__support/libc_errno.h"
1315
#include "src/__support/macros/config.h"
1416
#include "src/sys/mman/linux/shm_common.h"
1517

@@ -18,17 +20,19 @@ namespace LIBC_NAMESPACE_DECL {
1820
static constexpr int DEFAULT_OFLAGS = O_NOFOLLOW | O_CLOEXEC | O_NONBLOCK;
1921

2022
LLVM_LIBC_FUNCTION(int, shm_open, (const char *name, int oflags, mode_t mode)) {
21-
if (cpp::optional<shm_common::SHMPath> buffer =
22-
shm_common::translate_name(name)) {
23-
auto result = internal::open(buffer->data(), oflags | DEFAULT_OFLAGS, mode);
23+
auto path_result = shm_common::translate_name(name);
24+
if (!path_result.has_value()) {
25+
libc_errno = path_result.error();
26+
return -1;
27+
}
2428

25-
if (!result.has_value()) {
26-
libc_errno = result.error();
27-
return -1;
28-
}
29-
return result.value();
29+
auto open_result =
30+
internal::open(path_result->data(), oflags | DEFAULT_OFLAGS, mode);
31+
if (!open_result.has_value()) {
32+
libc_errno = open_result.error();
33+
return -1;
3034
}
31-
return -1;
35+
return open_result.value();
3236
}
3337

3438
} // namespace LIBC_NAMESPACE_DECL

‎libc/src/sys/mman/linux/shm_unlink.cpp‎

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,38 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "src/sys/mman/shm_unlink.h"
10+
11+
#include "hdr/fcntl_macros.h"
12+
#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
13+
#include "src/__support/libc_errno.h" // For internal errno.
1014
#include "src/__support/macros/config.h"
1115
#include "src/sys/mman/linux/shm_common.h"
12-
#include "src/unistd/unlink.h"
16+
#include <sys/syscall.h> // For SYS_unlink, SYS_unlinkat
1317

1418
namespace LIBC_NAMESPACE_DECL {
1519

16-
// TODO: stop calling the public unlink function. It should be calling an
17-
// internal shared utility.
20+
// TODO: move the unlink syscall to a shared utility.
1821

1922
LLVM_LIBC_FUNCTION(int, shm_unlink, (const char *name)) {
20-
if (cpp::optional<shm_common::SHMPath> buffer =
21-
shm_common::translate_name(name))
22-
return LIBC_NAMESPACE::unlink(buffer->data());
23-
return -1;
23+
auto path_result = shm_common::translate_name(name);
24+
if (!path_result.has_value()) {
25+
libc_errno = path_result.error();
26+
return -1;
27+
}
28+
#ifdef SYS_unlink
29+
int ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_unlink, path_result->data());
30+
#elif defined(SYS_unlinkat)
31+
int ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_unlinkat, AT_FDCWD,
32+
path_result->data(), 0);
33+
#else
34+
#error "unlink and unlinkat syscalls not available."
35+
#endif
36+
37+
if (ret < 0) {
38+
libc_errno = -ret;
39+
return -1;
40+
}
41+
return ret;
2442
}
2543

2644
} // namespace LIBC_NAMESPACE_DECL

‎libc/test/src/sys/mman/linux/madvise_test.cpp‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@
1313
#include "test/UnitTest/ErrnoSetterMatcher.h"
1414
#include "test/UnitTest/Test.h"
1515

16-
#include <sys/mman.h>
17-
1816
using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Fails;
1917
using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
2018
using LlvmLibcMadviseTest = LIBC_NAMESPACE::testing::ErrnoCheckingTest;

‎libc/test/src/sys/mman/linux/mincore_test.cpp‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
//
77
//===----------------------------------------------------------------------===//
88

9-
#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
109
#include "src/sys/mman/madvise.h"
1110
#include "src/sys/mman/mincore.h"
1211
#include "src/sys/mman/mlock.h"
@@ -18,10 +17,6 @@
1817
#include "test/UnitTest/ErrnoSetterMatcher.h"
1918
#include "test/UnitTest/Test.h"
2019

21-
#include <sys/mman.h>
22-
#include <sys/syscall.h>
23-
#include <unistd.h>
24-
2520
using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Fails;
2621
using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
2722
using LlvmLibcMincoreTest = LIBC_NAMESPACE::testing::ErrnoCheckingTest;

‎libc/test/src/sys/mman/linux/mlock_test.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66
//
77
//===----------------------------------------------------------------------===//
88

9+
// TODO: Simplify these tests and split them up. mlock, mlock2, mlockall,
10+
// munlock, and munlockall should have separate test files which only need to
11+
// check our code paths (succeeds and errors).
12+
913
#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
1014
#include "src/__support/libc_errno.h"
1115
#include "src/sys/mman/madvise.h"
@@ -24,10 +28,7 @@
2428
#include "test/UnitTest/Test.h"
2529

2630
#include <linux/capability.h>
27-
#include <sys/mman.h>
28-
#include <sys/resource.h>
2931
#include <sys/syscall.h>
30-
#include <unistd.h>
3132

3233
using namespace LIBC_NAMESPACE::testing::ErrnoSetterMatcher;
3334
using LlvmLibcMlockTest = LIBC_NAMESPACE::testing::ErrnoCheckingTest;

‎libc/test/src/sys/mman/linux/mremap_test.cpp‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@
1313
#include "test/UnitTest/ErrnoSetterMatcher.h"
1414
#include "test/UnitTest/Test.h"
1515

16-
#include <sys/mman.h>
17-
1816
using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Fails;
1917
using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
2018
using LlvmLibcMremapTest = LIBC_NAMESPACE::testing::ErrnoCheckingTest;

‎libc/test/src/sys/mman/linux/shm_test.cpp‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "hdr/fcntl_macros.h"
10-
#include "src/__support/OSUtil/syscall.h"
1110
#include "src/fcntl/fcntl.h"
1211
#include "src/sys/mman/mmap.h"
1312
#include "src/sys/mman/munmap.h"
@@ -18,7 +17,6 @@
1817
#include "test/UnitTest/ErrnoCheckingTest.h"
1918
#include "test/UnitTest/ErrnoSetterMatcher.h"
2019
#include "test/UnitTest/Test.h"
21-
#include <sys/syscall.h>
2220

2321
using namespace LIBC_NAMESPACE::testing::ErrnoSetterMatcher;
2422
using LlvmLibcShmTest = LIBC_NAMESPACE::testing::ErrnoCheckingTest;

0 commit comments

Comments
 (0)