Skip to content

Commit a506af4

Browse files
cdumezcdumez
committed
Add C++ wrappers for POSIX functions that take UTF-8 paths
https://bugs.webkit.org/show_bug.cgi?id=325773 Reviewed by Mike Wyrzykowski. Code that passed a UTF8CString to a POSIX file function had to unwrap it with legacyCStringPointer(). Add wtf/posix/POSIXExtras.h with wrappers that take a UTF8CStringView and keep the C return types: posixOpen(), posixAccess(), posixMkdir(), posixUnlink(), posixRename(), posixChmod(), posixSymlink(), posixStat(), posixLstat(), posixStatvfs(), posixOpendir() and posixRealpath(), plus posixChflags() on Darwin, posixStatx() on Linux, and posixDlopen(), posixShmOpen() and posixShmUnlink() everywhere but PlayStation. posixFopen() is available on every platform, including Windows, and takes an ASCIILiteral mode. Use the wrappers at the call sites in Source/ and Tools/, removing 38 calls to legacyCStringPointer(). Add a runtime/posix_string_wrappers style check that flags calls to the wrapped functions that pass legacyCStringPointer() directly as an argument, and suggests the wrapper instead. It shares its implementation with runtime/glib_string_wrappers. Both checks now treat a call qualified with a leading '::' as a call to the global function. * Source/JavaScriptCore/jit/ExecutableAllocator.cpp: * Source/JavaScriptCore/jit/GdbJIT.cpp: * Source/JavaScriptCore/jsc.cpp: * Source/JavaScriptCore/testRegExp.cpp: * Source/WTF/WTF.xcodeproj/project.pbxproj: * Source/WTF/wtf/CMakeLists.txt: * Source/WTF/wtf/darwin/LibraryPathDiagnostics.mm: * Source/WTF/wtf/playstation/FileSystemPlayStation.cpp: * Source/WTF/wtf/posix/FileSystemPOSIX.cpp: * Source/WTF/wtf/posix/POSIXExtras.h: Added. * Source/WebCore/page/linux/ResourceUsageThreadLinux.cpp: * Source/WebCore/platform/graphics/gbm/GBMDevice.cpp: * Source/WebCore/platform/graphics/vulkan/VulkanTypes.cpp: * Source/WebKit/NetworkProcess/cache/NetworkCacheIOChannelCocoa.mm: * Source/WebKit/Shared/Cocoa/SandboxExtensionCocoa.mm: * Source/WebKit/UIProcess/Cocoa/WebKitSwiftSoftLink.mm: * Source/WebKit/UIProcess/Notifications/glib/NotificationService.cpp: * Source/WebKit/UIProcess/linux/MemoryPressureMonitor.cpp: * Source/WebKit/WPEPlatform/wpe/wayland/WPEWaylandSHMPool.cpp: * Tools/Scripts/webkitpy/style/checkers/cpp.py: (_enclosing_function_call): (_check_string_wrappers): (check_glib_string_wrappers): (check_posix_string_wrappers): (check_style): * Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py: (WebKitStyleTest.test_posix_string_wrappers): * Tools/TestWebKitAPI/Tests/WTF/glib/FilePathWatcher.cpp: * Tools/TestWebKitAPI/Tests/WebCore/cocoa/DatabaseTrackerTest.mm: Canonical link: https://commits.webkit.org/322333@main
1 parent 3aa510d commit a506af4

23 files changed

Lines changed: 364 additions & 58 deletions

File tree

‎Source/JavaScriptCore/jit/ExecutableAllocator.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
#include <wtf/TZoneMallocInlines.h>
4848
#include <wtf/UUID.h>
4949
#include <wtf/WorkQueue.h>
50+
#include <wtf/posix/POSIXExtras.h>
5051

5152
#if ENABLE(LIBPAS_JIT_HEAP)
5253
WTF_ALLOW_UNSAFE_BUFFER_USAGE_BEGIN
@@ -1319,7 +1320,7 @@ void dumpJITMemory(const void* dst, const void* src, size_t size)
13191320
if (fd == -1) {
13201321
String path { Options::dumpJITMemoryPath() };
13211322
path = makeStringByReplacingAll(path, "%pid"_s, String::number(getCurrentProcessID()));
1322-
fd = open(FileSystem::fileSystemRepresentation(path).legacyCStringPointer(), O_CREAT | O_TRUNC | O_APPEND | O_WRONLY | O_EXLOCK | O_NONBLOCK, 0666);
1323+
fd = posixOpen(FileSystem::fileSystemRepresentation(path), O_CREAT | O_TRUNC | O_APPEND | O_WRONLY | O_EXLOCK | O_NONBLOCK, 0666);
13231324
RELEASE_ASSERT(fd != -1);
13241325
}
13251326
auto writeSpan = buffer->mutableSpan().first(offset);

‎Source/JavaScriptCore/jit/GdbJIT.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
#include <wtf/ProcessID.h>
4242
#include <wtf/RefCountedAndCanMakeWeakPtr.h>
4343
#include <wtf/StringPrintStream.h>
44+
#include <wtf/posix/POSIXExtras.h>
4445

4546
WTF_ALLOW_UNSAFE_BUFFER_USAGE_BEGIN
4647

@@ -1392,7 +1393,7 @@ static void addJITCodeEntry(GdbJITCodeMap& map, std::span<const uint8_t> region,
13921393
filename.print("/tmp");
13931394
filename.print("/jit-", getCurrentProcessID(), fileNum++, nameHint, ".o");
13941395
auto path = filename.toUTF8CString();
1395-
auto fd = open(path.legacyCStringPointer(), O_CREAT | O_TRUNC | O_RDWR, 0666);
1396+
auto fd = posixOpen(path, O_CREAT | O_TRUNC | O_RDWR, 0666);
13961397
RELEASE_ASSERT(fd != -1);
13971398
auto file = fdopen(fd, "wb");
13981399
RELEASE_ASSERT(file);

‎Source/JavaScriptCore/jsc.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@
113113
#include <wtf/URL.h>
114114
#include <wtf/WTFProcess.h>
115115
#include <wtf/WallTime.h>
116+
#include <wtf/posix/POSIXExtras.h>
116117
#include <wtf/text/Base64.h>
117118
#include <wtf/text/MakeString.h>
118119
#include <wtf/text/StringBuilder.h>
@@ -1327,7 +1328,7 @@ static RefPtr<Uint8Array> fillBufferWithContentsOfFile(FILE* file)
13271328

13281329
static RefPtr<Uint8Array> fillBufferWithContentsOfFile(const String& fileName)
13291330
{
1330-
FILE* f = fopen(fileName.utf8().legacyCStringPointer(), "rb");
1331+
FILE* f = posixFopen(fileName.utf8(), "rb"_s);
13311332
if (!f) {
13321333
SAFE_FPRINTF(stderr, "Could not open file: %s\n", fileName.utf8());
13331334
return nullptr;
@@ -1378,7 +1379,7 @@ static bool fillBufferWithContentsOfFile(const String& fileName, Vector<char>& b
13781379
SAFE_FPRINTF(stderr, "Trying to open a non-file: %s\n", *fileNameUTF);
13791380
return false;
13801381
}
1381-
auto* f = fopen(fileNameUTF->legacyCStringPointer(), "rb");
1382+
auto* f = posixFopen(*fileNameUTF, "rb"_s);
13821383
if (!f) {
13831384
SAFE_FPRINTF(stderr, "Could not open file: %s\n", *fileNameUTF);
13841385
return false;
@@ -1558,7 +1559,7 @@ static bool fetchModuleFromLocalFileSystem(const URL& fileURL, Vector& buffer)
15581559
if ((status.st_mode & S_IFMT) != S_IFREG)
15591560
return false;
15601561

1561-
FILE* f = fopen(pathName.legacyCStringPointer(), "r");
1562+
FILE* f = posixFopen(pathName, "r"_s);
15621563
#endif
15631564
if (!f) {
15641565
SAFE_FPRINTF(stderr, "Could not open file: %s\n", fileName.utf8());

‎Source/JavaScriptCore/testRegExp.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include <string.h>
3131
#include <wtf/Vector.h>
3232
#include <wtf/WTFProcess.h>
33+
#include <wtf/posix/POSIXExtras.h>
3334
#include <wtf/text/StringBuilder.h>
3435

3536
#if OS(WINDOWS)
@@ -393,7 +394,7 @@ static bool runFromFiles(GlobalObject* globalObject, const Vector<String>& files
393394

394395
bool success = true;
395396
for (size_t i = 0; i < files.size(); i++) {
396-
FILE* testCasesFile = fopen(files[i].utf8().legacyCStringPointer(), "rb");
397+
FILE* testCasesFile = posixFopen(files[i].utf8(), "rb"_s);
397398

398399
if (!testCasesFile) {
399400
SAFE_PRINTF("Unable to open test data file \"%s\"\n", files[i].utf8());

‎Source/WTF/WTF.xcodeproj/project.pbxproj‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@
137137
468378C62EC2D84200A9257A /* BOMSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = 468378C52EC2D84200A9257A /* BOMSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
138138
468742F12D7F8A0100F4BE74 /* FileHandle.h in Headers */ = {isa = PBXBuildFile; fileRef = 468742EF2D7F8A0100F4BE74 /* FileHandle.h */; settings = {ATTRIBUTES = (Private, ); }; };
139139
468742F22D7F8A0100F4BE74 /* FileHandle.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 468742F02D7F8A0100F4BE74 /* FileHandle.cpp */; };
140+
46F1A2C32E9B000000C0FFEE /* POSIXExtras.h in Headers */ = {isa = PBXBuildFile; fileRef = 46F1A2C22E9B000000C0FFEE /* POSIXExtras.h */; settings = {ATTRIBUTES = (Private, ); }; };
140141
468EAFD42D5B1E37005069CF /* SocketPOSIX.h in Headers */ = {isa = PBXBuildFile; fileRef = 468EAFD32D5B1E37005069CF /* SocketPOSIX.h */; settings = {ATTRIBUTES = (Private, ); }; };
141142
46A6D0D62D8A166D00FC76C8 /* MappedFileDataPOSIX.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 46A6D0D52D8A166D00FC76C8 /* MappedFileDataPOSIX.cpp */; };
142143
46A6D0DA2D8A168400FC76C8 /* MappedFileData.h in Headers */ = {isa = PBXBuildFile; fileRef = 46A6D0D72D8A168400FC76C8 /* MappedFileData.h */; settings = {ATTRIBUTES = (Private, ); }; };
@@ -1406,6 +1407,7 @@
14061407
468378C52EC2D84200A9257A /* BOMSPI.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = BOMSPI.h; sourceTree = "<group>"; };
14071408
468742EF2D7F8A0100F4BE74 /* FileHandle.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = FileHandle.h; sourceTree = "<group>"; };
14081409
468742F02D7F8A0100F4BE74 /* FileHandle.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = FileHandle.cpp; sourceTree = "<group>"; };
1410+
46F1A2C22E9B000000C0FFEE /* POSIXExtras.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = POSIXExtras.h; sourceTree = "<group>"; };
14091411
468EAFD32D5B1E37005069CF /* SocketPOSIX.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = SocketPOSIX.h; sourceTree = "<group>"; };
14101412
46A6D0D52D8A166D00FC76C8 /* MappedFileDataPOSIX.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = MappedFileDataPOSIX.cpp; sourceTree = "<group>"; };
14111413
46A6D0D72D8A168400FC76C8 /* MappedFileData.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MappedFileData.h; sourceTree = "<group>"; };
@@ -2331,6 +2333,7 @@
23312333
A331D96621F24ABD009F02AA /* FileSystemPOSIX.cpp */,
23322334
46A6D0D52D8A166D00FC76C8 /* MappedFileDataPOSIX.cpp */,
23332335
A3EE5C3921FFAC5E00FABD61 /* OSAllocatorPOSIX.cpp */,
2336+
46F1A2C22E9B000000C0FFEE /* POSIXExtras.h */,
23342337
468EAFD32D5B1E37005069CF /* SocketPOSIX.h */,
23352338
A32D8FA421FFFAB400780662 /* ThreadingPOSIX.cpp */,
23362339
);
@@ -3899,6 +3902,7 @@
38993902
DD3DC96F27A4BF8E007E5B61 /* PointerPreparations.h in Headers */,
39003903
FFE39CAC2B1D7472004972B0 /* policy.h in Headers */,
39013904
FFE39CB22B1D7472004972B0 /* policy_holder.h in Headers */,
3905+
46F1A2C32E9B000000C0FFEE /* POSIXExtras.h in Headers */,
39023906
8AB05DFC2C99482E00738BDD /* PreciseSum.h in Headers */,
39033907
DD3DC93227A4BF8E007E5B61 /* PrintStream.h in Headers */,
39043908
DD3DC95027A4BF8E007E5B61 /* PriorityQueue.h in Headers */,

‎Source/WTF/wtf/CMakeLists.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,8 @@ set(WTF_PUBLIC_HEADERS
467467
persistence/PersistentDecoder.h
468468
persistence/PersistentEncoder.h
469469

470+
posix/POSIXExtras.h
471+
470472
simde/simde.h
471473
simde/arm/neon.h
472474
simde/arm/sve.h

‎Source/WTF/wtf/darwin/LibraryPathDiagnostics.mm‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
#include <wtf/UUID.h>
4141
#include <wtf/cf/TypeCastsCF.h>
4242
#include <wtf/darwin/DispatchExtras.h>
43+
#include <wtf/posix/POSIXExtras.h>
4344
#include <wtf/spi/cf/CFPrivSPI.h>
4445
#include <wtf/spi/darwin/dyldSPI.h>
4546

@@ -173,7 +174,7 @@ static bool NODELETE isAddressInSharedRegion(const void* addr)
173174

174175
void LibraryPathDiagnosticsLogger::logDynamicLibraryInfo(const String& installName)
175176
{
176-
void *handle = dlopen(installName.utf8().legacyCStringPointer(), RTLD_NOLOAD);
177+
void *handle = posixDlopen(installName.utf8(), RTLD_NOLOAD);
177178
if (!handle)
178179
return;
179180

‎Source/WTF/wtf/playstation/FileSystemPlayStation.cpp‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include <dirent.h>
3434
#include <sys/statvfs.h>
3535
#include <wtf/Function.h>
36+
#include <wtf/posix/POSIXExtras.h>
3637
#include <wtf/text/MakeString.h>
3738

3839
namespace WTF {
@@ -47,9 +48,8 @@ static std::optional<FileType> fileTypePotentiallyFollowingSymLinks(const String
4748
if (fsRep.isEmpty())
4849
return std::nullopt;
4950

50-
auto statFunc = shouldFollowSymbolicLinks == ShouldFollowSymbolicLinks::Yes ? stat : lstat;
5151
struct stat fileInfo;
52-
if (statFunc(fsRep.legacyCStringPointer(), &fileInfo))
52+
if (shouldFollowSymbolicLinks == ShouldFollowSymbolicLinks::Yes ? posixStat(fsRep, &fileInfo) : posixLstat(fsRep, &fileInfo))
5353
return std::nullopt;
5454

5555
if (S_ISDIR(fileInfo.st_mode))
@@ -69,21 +69,21 @@ bool moveFile(const String& oldPath, const String& newPath)
6969
if (newFilename.isNull())
7070
return false;
7171

72-
return rename(oldFilename.legacyCStringPointer(), newFilename.legacyCStringPointer()) != -1;
72+
return posixRename(oldFilename, newFilename) != -1;
7373
}
7474

7575
std::optional<uint64_t> volumeFreeSpace(const String& path)
7676
{
7777
struct statvfs fileSystemStat;
78-
if (!statvfs(fileSystemRepresentation(path).legacyCStringPointer(), &fileSystemStat))
78+
if (!posixStatvfs(fileSystemRepresentation(path), &fileSystemStat))
7979
return fileSystemStat.f_bavail * fileSystemStat.f_frsize;
8080
return std::nullopt;
8181
}
8282

8383
std::optional<uint64_t> volumeCapacity(const String& path)
8484
{
8585
struct statvfs fileSystemStat;
86-
if (!statvfs(fileSystemRepresentation(path).legacyCStringPointer(), &fileSystemStat))
86+
if (!posixStatvfs(fileSystemRepresentation(path), &fileSystemStat))
8787
return fileSystemStat.f_blocks * fileSystemStat.f_frsize;
8888
return std::nullopt;
8989
}
@@ -92,7 +92,7 @@ Vector<String> listDirectorySub(const String& path, bool fullPath)
9292
{
9393
Vector<String> entries;
9494
auto cpath = fileSystemRepresentation(path);
95-
DIR* dir = opendir(cpath.legacyCStringPointer());
95+
DIR* dir = posixOpendir(cpath);
9696
if (dir) {
9797
struct dirent* dp;
9898
while ((dp = readdir(dir))) {
@@ -144,7 +144,7 @@ bool deleteNonEmptyDirectory(const String& path)
144144
String realPath(const String& filePath)
145145
{
146146
auto fsRep = fileSystemRepresentation(filePath);
147-
std::unique_ptr<char, decltype(free)*> resolvedPath(realpath(fsRep.legacyCStringPointer(), nullptr), free);
147+
std::unique_ptr<char, decltype(free)*> resolvedPath(posixRealpath(fsRep, nullptr), free);
148148
return resolvedPath ? String::fromUTF8(resolvedPath.get()) : filePath;
149149
}
150150

‎Source/WTF/wtf/posix/FileSystemPOSIX.cpp‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
#include <unistd.h>
3838
#include <wtf/FileHandle.h>
3939
#include <wtf/SafeStrerror.h>
40+
#include <wtf/posix/POSIXExtras.h>
4041
#include <wtf/text/CString.h>
4142
#include <wtf/text/MakeString.h>
4243
#include <wtf/text/StringBuilder.h>
@@ -89,7 +90,7 @@ FileHandle openFile(const String& path, FileOpenMode mode, FileAccessPermission
8990
else if (permission == FileAccessPermission::All)
9091
permissionFlag |= (S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH);
9192

92-
return FileHandle::adopt(open(fsRep.legacyCStringPointer(), platformFlag, permissionFlag), lockMode);
93+
return FileHandle::adopt(posixOpen(fsRep, platformFlag, permissionFlag), lockMode);
9394
}
9495

9596
std::optional<WallTime> fileCreationTime(const String& path)
@@ -102,7 +103,7 @@ std::optional<WallTime> fileCreationTime(const String& path)
102103
#if OS(LINUX) && HAVE(STATX)
103104
struct statx fileInfo;
104105

105-
if (statx(-1, fsRep.legacyCStringPointer(), 0, STATX_BTIME, &fileInfo) == -1)
106+
if (posixStatx(-1, fsRep, 0, STATX_BTIME, &fileInfo) == -1)
106107
return std::nullopt;
107108

108109
return WallTime::fromRawSeconds(fileInfo.stx_btime.tv_sec);
@@ -128,7 +129,7 @@ bool fileIDsAreEqual(std::optional<PlatformFileID> a, std::optional<PlatformFile
128129
std::optional<uint32_t> volumeFileBlockSize(const String& path)
129130
{
130131
struct statvfs fileStat;
131-
if (!statvfs(fileSystemRepresentation(path).legacyCStringPointer(), &fileStat))
132+
if (!posixStatvfs(fileSystemRepresentation(path), &fileStat))
132133
return fileStat.f_frsize;
133134

134135
return std::nullopt;
@@ -210,13 +211,13 @@ std::optional<int32_t> getFileDeviceId(const String& path)
210211

211212
bool fileExists(const String& path)
212213
{
213-
return access(fileSystemRepresentation(path).legacyCStringPointer(), F_OK) != -1;
214+
return posixAccess(fileSystemRepresentation(path), F_OK) != -1;
214215
}
215216

216217
bool deleteFile(const String& path)
217218
{
218219
// unlink(...) returns 0 on successful deletion of the path and non-zero in any other case (including invalid permissions or non-existent file)
219-
bool unlinked = !unlink(fileSystemRepresentation(path).legacyCStringPointer());
220+
bool unlinked = !posixUnlink(fileSystemRepresentation(path));
220221
if (!unlinked && errno != ENOENT)
221222
LOG_ERROR("File failed to delete. Error message: %s", safeStrerror(errno));
222223

@@ -230,7 +231,7 @@ bool makeAllDirectories(const String& path)
230231
if (!length)
231232
return false;
232233

233-
if (!access(fullPath.legacyCStringPointer(), F_OK))
234+
if (!posixAccess(fullPath, F_OK))
234235
return true;
235236

236237
auto p = byteCast<char>(fullPath.mutableSpanIncludingNullTerminator()).subspan(1);
@@ -239,15 +240,15 @@ bool makeAllDirectories(const String& path)
239240
for (; p[0]; skip(p, 1)) {
240241
if (p[0] == '/') {
241242
p[0] = '\0';
242-
if (access(fullPath.legacyCStringPointer(), F_OK)) {
243-
if (mkdir(fullPath.legacyCStringPointer(), S_IRWXU))
243+
if (posixAccess(fullPath, F_OK)) {
244+
if (posixMkdir(fullPath, S_IRWXU))
244245
return false;
245246
}
246247
p[0] = '/';
247248
}
248249
}
249-
if (access(fullPath.legacyCStringPointer(), F_OK)) {
250-
if (mkdir(fullPath.legacyCStringPointer(), S_IRWXU))
250+
if (posixAccess(fullPath, F_OK)) {
251+
if (posixMkdir(fullPath, S_IRWXU))
251252
return false;
252253
}
253254

0 commit comments

Comments
 (0)