diff options
author | Ben Wagner <bungeman@google.com> | 2017-03-10 13:08:15 -0500 |
---|---|---|
committer | Skia Commit-Bot <skia-commit-bot@chromium.org> | 2017-03-10 19:58:46 +0000 |
commit | 4d1955c43aaab045511b74a495dfbea4ef0057c5 (patch) | |
tree | 23d0f457f69b98bde35dc9a3afa32451ee5f6694 | |
parent | dc9f0dbe4cdcdf6fead5fc28532d58f7d998a447 (diff) |
Fix SkFILEStream.
Change-Id: I8c66e4e3e857227aed3d0bc497982f4c0d96d917
Reviewed-on: https://skia-review.googlesource.com/9498
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
-rw-r--r-- | include/core/SkOSFile.h | 13 | ||||
-rw-r--r-- | include/core/SkStream.h | 36 | ||||
-rw-r--r-- | src/codec/SkIcoCodec.h | 1 | ||||
-rw-r--r-- | src/core/SkPath.cpp | 1 | ||||
-rw-r--r-- | src/core/SkStream.cpp | 116 | ||||
-rw-r--r-- | src/pdf/SkPDFConvertType1FontStream.cpp | 1 | ||||
-rw-r--r-- | src/ports/SkImageEncoder_WIC.cpp | 1 | ||||
-rw-r--r-- | src/ports/SkOSFile_posix.cpp | 12 | ||||
-rw-r--r-- | src/ports/SkOSFile_stdio.cpp | 50 | ||||
-rw-r--r-- | src/ports/SkOSFile_win.cpp | 27 | ||||
-rw-r--r-- | src/utils/SkShadowUtils.cpp | 1 | ||||
-rw-r--r-- | tests/PathOpsExtendedTest.cpp | 2 | ||||
-rw-r--r-- | tests/StreamTest.cpp | 2 | ||||
-rw-r--r-- | tools/chrome_fuzz.cpp | 4 |
14 files changed, 124 insertions, 143 deletions
diff --git a/include/core/SkOSFile.h b/include/core/SkOSFile.h index 876135596b..88ad958b1c 100644 --- a/include/core/SkOSFile.h +++ b/include/core/SkOSFile.h @@ -24,20 +24,12 @@ FILE* sk_fopen(const char path[], SkFILE_Flags); void sk_fclose(FILE*); size_t sk_fgetsize(FILE*); -/** Return true if the file could seek back to the beginning -*/ -bool sk_frewind(FILE*); -size_t sk_fread(void* buffer, size_t byteCount, FILE*); size_t sk_fwrite(const void* buffer, size_t byteCount, FILE*); -char* sk_fgets(char* str, int size, FILE* f); - void sk_fflush(FILE*); void sk_fsync(FILE*); -bool sk_fseek(FILE*, size_t); -bool sk_fmove(FILE*, long); size_t sk_ftell(FILE*); /** Maps a file into memory. Returns the address and length on success, NULL otherwise. @@ -73,8 +65,9 @@ bool sk_exists(const char *path, SkFILE_Flags = (SkFILE_Flags)0); // Returns true if a directory exists at this path. bool sk_isdir(const char *path); -// Have we reached the end of the file? -int sk_feof(FILE *); +// Like pread, but may affect the file position marker. +// Returns the number of bytes read or SIZE_MAX if failed. +size_t sk_qread(FILE*, void* buffer, size_t count, size_t offset); // Create a new directory at this path; returns true if successful. diff --git a/include/core/SkStream.h b/include/core/SkStream.h index 419ef7b723..10929a8343 100644 --- a/include/core/SkStream.h +++ b/include/core/SkStream.h @@ -12,6 +12,8 @@ #include "SkRefCnt.h" #include "SkScalar.h" +#include <memory.h> + class SkStream; class SkStreamRewindable; class SkStreamSeekable; @@ -232,7 +234,6 @@ public: //////////////////////////////////////////////////////////////////////////////////////// -#include "SkString.h" #include <stdio.h> /** A stream that wraps a C FILE* file stream. */ @@ -241,28 +242,20 @@ public: /** Initialize the stream by calling sk_fopen on the specified path. * This internal stream will be closed in the destructor. */ - explicit SkFILEStream(const char path[] = NULL); + explicit SkFILEStream(const char path[] = nullptr); - enum Ownership { - kCallerPasses_Ownership, - kCallerRetains_Ownership - }; /** Initialize the stream with an existing C file stream. - * While this stream exists, it assumes exclusive access to the C file stream. - * The C file stream will be closed in the destructor unless the caller specifies - * kCallerRetains_Ownership. + * The C file stream will be closed in the destructor. */ - explicit SkFILEStream(FILE* file, Ownership ownership = kCallerPasses_Ownership); + explicit SkFILEStream(FILE* file); virtual ~SkFILEStream(); /** Returns true if the current path could be opened. */ - bool isValid() const { return fFILE != NULL; } + bool isValid() const { return fFILE != nullptr; } - /** Close the current file, and open a new file with the specified path. - * If path is NULL, just close the current file. - */ - void setPath(const char path[]); + /** Close this SkFILEStream. */ + void close(); size_t read(void* buffer, size_t size) override; bool isAtEnd() const override; @@ -280,11 +273,14 @@ public: const void* getMemoryBase() override; private: - FILE* fFILE; - SkString fName; - Ownership fOwnership; - // fData is lazilly initialized when needed. - mutable sk_sp<SkData> fData; + explicit SkFILEStream(std::shared_ptr<FILE>, size_t size, size_t offset); + explicit SkFILEStream(std::shared_ptr<FILE>, size_t size, size_t offset, size_t originalOffset); + + std::shared_ptr<FILE> fFILE; + // My own council will I keep on sizes and offsets. + size_t fSize; + size_t fOffset; + size_t fOriginalOffset; typedef SkStreamAsset INHERITED; }; diff --git a/src/codec/SkIcoCodec.h b/src/codec/SkIcoCodec.h index f9a14eb377..2a4efb50b0 100644 --- a/src/codec/SkIcoCodec.h +++ b/src/codec/SkIcoCodec.h @@ -10,6 +10,7 @@ #include "SkCodec.h" #include "SkImageInfo.h" #include "SkStream.h" +#include "SkTArray.h" #include "SkTypes.h" /* diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp index 0cb5853b77..f4c8e8b7c4 100644 --- a/src/core/SkPath.cpp +++ b/src/core/SkPath.cpp @@ -2111,6 +2111,7 @@ size_t SkPath::readFromMemory(const void* storage, size_t length) { /////////////////////////////////////////////////////////////////////////////// +#include "SkString.h" #include "SkStringUtils.h" #include "SkStream.h" diff --git a/src/core/SkStream.cpp b/src/core/SkStream.cpp index db9caee541..1435dab235 100644 --- a/src/core/SkStream.cpp +++ b/src/core/SkStream.cpp @@ -155,105 +155,95 @@ bool SkWStream::writeStream(SkStream* stream, size_t length) { /////////////////////////////////////////////////////////////////////////////// -SkFILEStream::SkFILEStream(const char file[]) : fName(file), fOwnership(kCallerPasses_Ownership) { - fFILE = file ? sk_fopen(fName.c_str(), kRead_SkFILE_Flag) : nullptr; -} +SkFILEStream::SkFILEStream(std::shared_ptr<FILE> file, size_t size, + size_t offset, size_t originalOffset) + : fFILE(std::move(file)) + , fSize(size) + , fOffset(SkTMin(offset, fSize)) + , fOriginalOffset(SkTMin(originalOffset, fSize)) +{ } + +SkFILEStream::SkFILEStream(std::shared_ptr<FILE> file, size_t size, size_t offset) + : SkFILEStream(std::move(file), size, offset, offset) +{ } + +SkFILEStream::SkFILEStream(FILE* file) + : SkFILEStream(std::shared_ptr<FILE>(file, sk_fclose), + file ? sk_fgetsize(file) : 0, + file ? sk_ftell(file) : 0) +{ } -SkFILEStream::SkFILEStream(FILE* file, Ownership ownership) - : fFILE(file) - , fOwnership(ownership) { -} + +SkFILEStream::SkFILEStream(const char path[]) + : SkFILEStream(path ? sk_fopen(path, kRead_SkFILE_Flag) : nullptr) +{ } SkFILEStream::~SkFILEStream() { - if (fFILE && fOwnership != kCallerRetains_Ownership) { - sk_fclose(fFILE); - } + this->close(); } -void SkFILEStream::setPath(const char path[]) { - fName.set(path); - if (fFILE) { - sk_fclose(fFILE); - fFILE = nullptr; - } - if (path) { - fFILE = sk_fopen(fName.c_str(), kRead_SkFILE_Flag); - } +void SkFILEStream::close() { + fFILE.reset(); + fSize = 0; + fOffset = 0; } size_t SkFILEStream::read(void* buffer, size_t size) { - if (fFILE) { - return sk_fread(buffer, size, fFILE); + if (size > fSize - fOffset) { + size = fSize - fOffset; } - return 0; + size_t bytesRead = size; + if (buffer) { + bytesRead = sk_qread(fFILE.get(), buffer, size, fOffset); + } + if (bytesRead == SIZE_MAX) { + return 0; + } + fOffset += bytesRead; + return bytesRead; } bool SkFILEStream::isAtEnd() const { - return sk_feof(fFILE); + if (fOffset == fSize) { + return true; + } + return fOffset >= sk_fgetsize(fFILE.get()); } bool SkFILEStream::rewind() { - if (fFILE) { - if (sk_frewind(fFILE)) { - return true; - } - // we hit an error - sk_fclose(fFILE); - fFILE = nullptr; - } - return false; + // TODO: fOriginalOffset instead of 0. + fOffset = 0; + return true; } SkStreamAsset* SkFILEStream::duplicate() const { - if (nullptr == fFILE) { - return new SkMemoryStream(); - } - - if (fData.get()) { - return new SkMemoryStream(fData); - } - - if (!fName.isEmpty()) { - std::unique_ptr<SkFILEStream> that(new SkFILEStream(fName.c_str())); - if (sk_fidentical(that->fFILE, this->fFILE)) { - return that.release(); - } - } - - fData = SkData::MakeFromFILE(fFILE); - if (nullptr == fData) { - return nullptr; - } - return new SkMemoryStream(fData); + // TODO: fOriginalOffset instead of 0. + return new SkFILEStream(fFILE, fSize, 0, fOriginalOffset); } size_t SkFILEStream::getPosition() const { - return sk_ftell(fFILE); + return fOffset; } bool SkFILEStream::seek(size_t position) { - return sk_fseek(fFILE, position); + fOffset = position > fSize ? fSize : position; + return true; } bool SkFILEStream::move(long offset) { - return sk_fmove(fFILE, offset); + return this->seek(fOffset + offset); } SkStreamAsset* SkFILEStream::fork() const { - std::unique_ptr<SkStreamAsset> that(this->duplicate()); - that->seek(this->getPosition()); - return that.release(); + return new SkFILEStream(fFILE, fSize, fOffset, fOriginalOffset); } size_t SkFILEStream::getLength() const { - return sk_fgetsize(fFILE); + return fSize; } const void* SkFILEStream::getMemoryBase() { - if (nullptr == fData.get()) { - return nullptr; - } - return fData->data(); + return nullptr; } /////////////////////////////////////////////////////////////////////////////// diff --git a/src/pdf/SkPDFConvertType1FontStream.cpp b/src/pdf/SkPDFConvertType1FontStream.cpp index d75da5c787..387f0efacf 100644 --- a/src/pdf/SkPDFConvertType1FontStream.cpp +++ b/src/pdf/SkPDFConvertType1FontStream.cpp @@ -6,6 +6,7 @@ */ #include "SkPDFConvertType1FontStream.h" +#include "SkTemplates.h" #include <ctype.h> diff --git a/src/ports/SkImageEncoder_WIC.cpp b/src/ports/SkImageEncoder_WIC.cpp index 6bc8b5473e..6d355c1d47 100644 --- a/src/ports/SkImageEncoder_WIC.cpp +++ b/src/ports/SkImageEncoder_WIC.cpp @@ -36,6 +36,7 @@ #include "SkImageEncoder.h" #include "SkStream.h" #include "SkTScopedComPtr.h" +#include "SkTemplates.h" #include "SkUnPreMultiply.h" #include <wincodec.h> diff --git a/src/ports/SkOSFile_posix.cpp b/src/ports/SkOSFile_posix.cpp index 396de68bbe..48b5b95ad3 100644 --- a/src/ports/SkOSFile_posix.cpp +++ b/src/ports/SkOSFile_posix.cpp @@ -95,6 +95,18 @@ void* sk_fmmap(FILE* f, size_t* size) { return sk_fdmmap(fd, size); } +size_t sk_qread(FILE* file, void* buffer, size_t count, size_t offset) { + int fd = sk_fileno(file); + if (fd < 0) { + return SIZE_MAX; + } + ssize_t bytesRead = pread(fd, buffer, count, offset); + if (bytesRead < 0) { + return SIZE_MAX; + } + return bytesRead; +} + //////////////////////////////////////////////////////////////////////////// struct SkOSFileIterData { diff --git a/src/ports/SkOSFile_stdio.cpp b/src/ports/SkOSFile_stdio.cpp index 1c4bd4babd..68c2d3d4d7 100644 --- a/src/ports/SkOSFile_stdio.cpp +++ b/src/ports/SkOSFile_stdio.cpp @@ -87,15 +87,6 @@ FILE* sk_fopen(const char path[], SkFILE_Flags flags) { return file; } -char* sk_fgets(char* str, int size, FILE* f) { - return fgets(str, size, (FILE *)f); -} - -int sk_feof(FILE *f) { - // no :: namespace qualifier because it breaks android - return feof((FILE *)f); -} - size_t sk_fgetsize(FILE* f) { SkASSERT(f); @@ -114,32 +105,6 @@ size_t sk_fgetsize(FILE* f) { return size; } -bool sk_frewind(FILE* f) { - SkASSERT(f); - ::rewind(f); - return true; -} - -size_t sk_fread(void* buffer, size_t byteCount, FILE* f) { - SkASSERT(f); - if (buffer == nullptr) { - size_t curr = ftell(f); - if ((long)curr == -1) { - SkDEBUGF(("sk_fread: ftell(%p) returned -1 feof:%d ferror:%d\n", f, feof(f), ferror(f))); - return 0; - } - int err = fseek(f, (long)byteCount, SEEK_CUR); - if (err != 0) { - SkDEBUGF(("sk_fread: fseek(%d) tell:%d failed with feof:%d ferror:%d returned:%d\n", - byteCount, curr, feof(f), ferror(f), err)); - return 0; - } - return byteCount; - } - else - return fread(buffer, 1, byteCount, f); -} - size_t sk_fwrite(const void* buffer, size_t byteCount, FILE* f) { SkASSERT(f); return fwrite(buffer, 1, byteCount, f); @@ -158,16 +123,6 @@ void sk_fsync(FILE* f) { #endif } -bool sk_fseek(FILE* f, size_t byteCount) { - int err = fseek(f, (long)byteCount, SEEK_SET); - return err == 0; -} - -bool sk_fmove(FILE* f, long byteCount) { - int err = fseek(f, byteCount, SEEK_CUR); - return err == 0; -} - size_t sk_ftell(FILE* f) { long curr = ftell(f); if (curr < 0) { @@ -177,8 +132,9 @@ size_t sk_ftell(FILE* f) { } void sk_fclose(FILE* f) { - SkASSERT(f); - fclose(f); + if (f) { + fclose(f); + } } bool sk_isdir(const char *path) { diff --git a/src/ports/SkOSFile_win.cpp b/src/ports/SkOSFile_win.cpp index cf46cea985..ceafc7904f 100644 --- a/src/ports/SkOSFile_win.cpp +++ b/src/ports/SkOSFile_win.cpp @@ -124,6 +124,33 @@ void* sk_fmmap(FILE* f, size_t* length) { return sk_fdmmap(fileno, length); } +size_t sk_qread(FILE* file, void* buffer, size_t count, size_t offset) { + int fileno = sk_fileno(file); + HANDLE fileHandle = (HANDLE)_get_osfhandle(fileno); + if (INVALID_HANDLE_VALUE == file) { + return SIZE_MAX; + } + + OVERLAPPED overlapped = {0}; + ULARGE_INTEGER winOffset; + winOffset.QuadPart = offset; + overlapped.Offset = winOffset.LowPart; + overlapped.OffsetHigh = winOffset.HighPart; + + if (!SkTFitsIn<DWORD>(count)) { + count = std::numeric_limits<DWORD>::max(); + } + + DWORD bytesRead; + if (ReadFile(fileHandle, buffer, static_cast<DWORD>(count), &bytesRead, &overlapped)) { + return bytesRead; + } + if (GetLastError() == ERROR_HANDLE_EOF) { + return 0; + } + return SIZE_MAX; +} + //////////////////////////////////////////////////////////////////////////// struct SkOSFileIterData { diff --git a/src/utils/SkShadowUtils.cpp b/src/utils/SkShadowUtils.cpp index 94dd113a5f..eb015aa8f9 100644 --- a/src/utils/SkShadowUtils.cpp +++ b/src/utils/SkShadowUtils.cpp @@ -12,6 +12,7 @@ #include "SkRandom.h" #include "SkResourceCache.h" #include "SkShadowTessellator.h" +#include "SkString.h" #include "SkTLazy.h" #include "SkVertices.h" #if SK_SUPPORT_GPU diff --git a/tests/PathOpsExtendedTest.cpp b/tests/PathOpsExtendedTest.cpp index 279faa422c..8f24adf3ba 100644 --- a/tests/PathOpsExtendedTest.cpp +++ b/tests/PathOpsExtendedTest.cpp @@ -616,7 +616,7 @@ void initializeTests(skiatest::Reporter* reporter, const char* test) { inData.setCount((int) inFile.getLength()); size_t inLen = inData.count(); inFile.read(inData.begin(), inLen); - inFile.setPath(nullptr); + inFile.close(); char* insert = strstr(inData.begin(), marker); if (insert) { insert += sizeof(marker) - 1; diff --git a/tests/StreamTest.cpp b/tests/StreamTest.cpp index fba09c989a..2e476a1858 100644 --- a/tests/StreamTest.cpp +++ b/tests/StreamTest.cpp @@ -69,7 +69,7 @@ static void test_filestreams(skiatest::Reporter* reporter, const char* tmpDir) { { FILE* file = ::fopen(path.c_str(), "rb"); - SkFILEStream stream(file, SkFILEStream::kCallerPasses_Ownership); + SkFILEStream stream(file); REPORTER_ASSERT(reporter, stream.isValid()); test_loop_stream(reporter, &stream, s, 26, 100); diff --git a/tools/chrome_fuzz.cpp b/tools/chrome_fuzz.cpp index 94479662b4..fcc0db2f4c 100644 --- a/tools/chrome_fuzz.cpp +++ b/tools/chrome_fuzz.cpp @@ -8,6 +8,8 @@ #include "SkOSFile.h" #include "SkString.h" +#include <stdio.h> + static const int kBitmapSize = 24; static bool read_test_case(const char* filename, SkString* testdata) { @@ -22,7 +24,7 @@ static bool read_test_case(const char* filename, SkString* testdata) { return false; } testdata->resize(len); - (void) sk_fread(testdata->writable_str(), len, file); + (void) fread(testdata->writable_str(), len, file); return true; } |