[Webkit-unassigned] [Bug 194344] [WTF] Add getting/changing current working directory to FileSystem

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=194344

--- Comment #12 from Darin Adler <darin at apple.com> ---
Comment on attachment 361645
  --> https://bugs.webkit.org/attachment.cgi?id=361645
Add get/change current working directory functions, with JSC changes

View in context: https://bugs.webkit.org/attachment.cgi?id=361645&action=review

Still not sure that we need a working directory abstraction. If we want "jsc" to have a working directory concept we can simulate that with a global string inside the tool on all platforms, not just PlayStation. No reason we have to use the "real" working directory, except to initialize the tool on startup.

> Source/JavaScriptCore/jsc.cpp:889
> +    FileSystem::PlatformFileHandle handle = FileSystem::openFile(fileName, FileSystem::FileOpenMode::Read);

Here’s a place where "auto" can help a bit. Nice to not have to write out FileSystem::PlatformFileHandle.

> Source/JavaScriptCore/jsc.cpp:902
> +static bool fillBufferWithContentsOfFile(FileSystem::PlatformFileHandle &handle, Vector& buffer)

This function is pretty dangerous in the case where the vector is not of a type that is a single byte.

> Source/JavaScriptCore/jsc.cpp:912
>      buffer.resize(bufferCapacity + initialSize);

Should be grow instead of resize for better efficiency.

> Source/JavaScriptCore/jsc.cpp:915
> +    int readSize = FileSystem::readFromFile(handle, data, bufferCapacity);

I’d suggest auto for readSize.

> Source/JavaScriptCore/jsc.cpp:2753
> +        static const unsigned workingDirectoryStrLength = strlen("--working-directory=");
> +        if (!strncmp(arg, "--working-directory=", workingDirectoryStrLength)) {
> +            m_workingDirectory = String(arg + workingDirectoryStrLength);
> +            continue;
> +        }

Could add a comment here explaining this option is provided primarily for the benefit of platforms that don’t have a working directory concept at the shell level (PlayStation).

> Source/WTF/wtf/FileSystem.h:190
> +WTF_EXPORT_PRIVATE Optional<String> getCurrentWorkingDirectory();

WebKit coding style means we would not use the word "get" in the name of this function. It should be called currentWorkingDirectory or workingDirectory.

> Source/WTF/wtf/FileSystem.h:191
> +WTF_EXPORT_PRIVATE bool changeCurrentWorkingDirectory(const String&);

Not 100% sure we need the word "current" in this function name.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20190211/c5d22719/attachment.html>