[Webkit-unassigned] [Bug 36938] Add basic FileSystem operations for FileReader/FileWriter support

[bugzilla-daemon at webkit.org]

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


Jian Li <jianli at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #52460|review?                     |review-
               Flag|                            |




--- Comment #7 from Jian Li <jianli at chromium.org>  2010-04-05 10:46:52 PST ---
(From update of attachment 52460)
> --- a/WebCore/platform/FileSystem.h
> +++ b/WebCore/platform/FileSystem.h
> @@ -141,8 +152,12 @@ inline bool isHandleValid(const PlatformFileHandle& handle) { return handle != i
>  
>  // Prefix is what the filename should be prefixed with, not the full path.
>  WTF::CString openTemporaryFile(const char* prefix, PlatformFileHandle&);
> +PlatformFileHandle openFile(const String& path, FileOpenMode);
>  void closeFile(PlatformFileHandle&);
> +long long seekFile(PlatformFileHandle, long long offset, FileSeekOrigin);
> +bool truncateFile(PlatformFileHandle, long long offset);
>  int writeToFile(PlatformFileHandle, const char* data, int length);
> +int readFromFile(PlatformFileHandle, char* data, int length);
Better to add comment saying that return value less than 0 indicates an error.
>  
>  // Methods for dealing with loadable modules
>  bool unloadModule(PlatformModule);
> diff --git a/WebCore/platform/posix/FileSystemPOSIX.cpp b/WebCore/platform/posix/FileSystemPOSIX.cpp
> index d6804fb..57a36b8 100644
> --- a/WebCore/platform/posix/FileSystemPOSIX.cpp
> +++ b/WebCore/platform/posix/FileSystemPOSIX.cpp
> +void closeFile(PlatformFileHandle& handle)
> +{
> +    close(handle);
Since handle parameter is passed by reference, you need to set it to
invalidPlatformFileHandle if closing is successful, like the following:
    if (isHandleValid(handle)) {
        close(handle);
        handle = invalidPlatformFileHandle;
    }
> +}
> +
> +long long seekFile(PlatformFileHandle handle, long long offset, FileSeekOrigin origin)
> +{
> +    int whence = SEEK_SET;
> +    switch (origin) {
> +    case SeekFromBeginning: whence = SEEK_SET; break;
Per WebKit coding style, each statement should get its own line.
> +    case SeekFromCurrent: whence = SEEK_CUR; break;
ditto.
> +    case SeekFromEnd: whence = SEEK_END; break;
ditto.
> +    default: ASSERT_NOT_REACHED();
ditto.
> +    }
> +    return static_cast<long long>(lseek(handle, offset, whence));
> +}
> +
> +bool truncateFile(PlatformFileHandle handle, long long offset)
> +{
> +    if (ftruncate(handle, offset) < 0)
> +        return false;
> +    return true;
Simpler to say:
       return ftruncate(handle, offset) == 0;
> +}
> +
> +int writeToFile(PlatformFileHandle handle, const char* data, int length)
> +{
> +    int totalBytesWritten = 0;
> +    while (totalBytesWritten < length) {
> +        int bytesWritten = HANDLE_EINTR(write(handle, data + totalBytesWritten, length - totalBytesWritten));
Using HANDLE_EINTR might not be necessary and it causes the complexity to
understand the code.
How about something like the following:
           int bytesWritten = write(handle, data + totalBytesWritten,
static_cast<size_t>(length - totalBytesWritten));
           if (bytesWritten < 0 && errno != EINTR)
               return -1;
In addition, better to do static_cast to size_t from int.
> +        if (bytesWritten < 0)
> +            return -1;
> +        totalBytesWritten += bytesWritten;
> +    }
> +
> +    return totalBytesWritten;
> +}
> +
> +int readFromFile(PlatformFileHandle handle, char* data, int length)
> +{
> +    int totalBytesRead = 0;
> +    while (totalBytesRead < length) {
> +        int bytesRead = HANDLE_EINTR(read(handle, data + totalBytesRead, length - totalBytesRead));
ditto.
> +        if (bytesRead <= 0)
> +            break;
> +        totalBytesRead += bytesRead;
> +    }
> +
> +    return totalBytesRead;
> +}
> +
>  bool deleteEmptyDirectory(const String& path)
>  {
>      CString fsRep = fileSystemRepresentation(path);

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.