Fix SkFILEStream.
Cherry-picked from upstream Skia:
4d1955c43aaab045511b74a495dfbea4ef0057c5
Differences:
- include <memory> instead of <memory.h>, to compile.
- re-include "SkString.h", so we don't have to fix downstream
clients that do not IWYU.
- Stop building SkRTConf and its test. SkRTConf uses code that
this patch deletes in SkOSFile. SkRTConf itself is deleted in
upstream's 4e44efe50474d4eebcb30b762e784b3ef2126750. For
simplicity in cherry-picking further back this CL does not
delete the code, but simply disables it by not building it.
(It is only used if SK_DEVELOPER is defined anyway, which
is not the case on Android.)
Bug: 65646012
Bug: 65426286
Test: I863385d797d7a6c54e37904b4f023ff694e01785
(Original) Change-Id: I8c66e4e3e857227aed3d0bc497982f4c0d96d917
Merged-In: I220ec2e2e83f4a002846e89dce855ed5926ca4a1
Merged-In: Ia36e16282eaf294709ce41f57a0f40fe887c1546
Merged-In: Ief1b80a012affeda2068b70405ab1a9f08b36867
Reviewed-on: https://skia-review.googlesource.com/9498
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Conflicts:
include/core/SkStream.h
src/core/SkStream.cpp
src/pdf/SkPDFConvertType1FontStream.cpp
src/ports/SkImageEncoder_WIC.cpp
src/utils/SkShadowUtils.cpp
Change-Id: Ib7aaf367f68f8764147887d858f65ee14fa7a1d9
Exempt-From-Owner-Approval: I should be an OWNER
(cherry picked from commit 67f9bd2acfd17f64a33ae8ad14806a0c93b921d8)
Change-Id: I7d1142040b3c62e764a726a82a1fd8850dab1458
diff --git a/Android.mk b/Android.mk
index 77c258a..4dbd17bb 100644
--- a/Android.mk
+++ b/Android.mk
@@ -643,7 +643,6 @@
src/utils/SkPatchGrid.cpp \
src/utils/SkPatchUtils.cpp \
src/utils/SkRGBAToYUV.cpp \
- src/utils/SkRTConf.cpp \
src/utils/SkTextBox.cpp \
src/utils/SkTextureCompressor.cpp \
src/utils/SkTextureCompressor_ASTC.cpp \
diff --git a/dm/Android.mk b/dm/Android.mk
index e51da0f..f362c99 100644
--- a/dm/Android.mk
+++ b/dm/Android.mk
@@ -213,7 +213,6 @@
../tests/PremulAlphaRoundTripTest.cpp \
../tests/QuickRejectTest.cpp \
../tests/RRectInPathTest.cpp \
- ../tests/RTConfRegistryTest.cpp \
../tests/RTreeTest.cpp \
../tests/RandomTest.cpp \
../tests/ReadPixelsTest.cpp \
diff --git a/include/core/SkOSFile.h b/include/core/SkOSFile.h
index f977327..cb775e2 100644
--- a/include/core/SkOSFile.h
+++ b/include/core/SkOSFile.h
@@ -31,20 +31,12 @@
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.
@@ -80,8 +72,9 @@
// 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 4502416..265fb64 100644
--- a/include/core/SkStream.h
+++ b/include/core/SkStream.h
@@ -11,8 +11,9 @@
#include "SkRefCnt.h"
#include "SkScalar.h"
-class SkData;
+#include <memory>
+class SkData;
class SkStream;
class SkStreamRewindable;
class SkStreamSeekable;
@@ -231,28 +232,20 @@
/** 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;
@@ -270,11 +263,14 @@
const void* getMemoryBase() override;
private:
- FILE* fFILE;
- SkString fName;
- Ownership fOwnership;
- // fData is lazilly initialized when needed.
- mutable SkAutoTUnref<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 9a3f248..5a3740d 100644
--- a/src/codec/SkIcoCodec.h
+++ b/src/codec/SkIcoCodec.h
@@ -8,6 +8,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 320448a..9eb5cd1 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -2082,6 +2082,7 @@
///////////////////////////////////////////////////////////////////////////////
+#include "SkString.h"
#include "SkStringUtils.h"
#include "SkStream.h"
diff --git a/src/core/SkStream.cpp b/src/core/SkStream.cpp
index 9529308..4f00768 100644
--- a/src/core/SkStream.cpp
+++ b/src/core/SkStream.cpp
@@ -180,105 +180,95 @@
///////////////////////////////////////////////////////////////////////////////
-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(FILE* file, Ownership ownership)
- : fFILE(file)
- , fOwnership(ownership) {
-}
+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(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()) {
- SkAutoTDelete<SkFILEStream> that(new SkFILEStream(fName.c_str()));
- if (sk_fidentical(that->fFILE, this->fFILE)) {
- return that.detach();
- }
- }
-
- fData.reset(SkData::NewFromFILE(fFILE));
- if (nullptr == fData.get()) {
- 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 {
- SkAutoTDelete<SkStreamAsset> that(this->duplicate());
- that->seek(this->getPosition());
- return that.detach();
+ 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/ports/SkOSFile_posix.cpp b/src/ports/SkOSFile_posix.cpp
index 396de68..48b5b95 100644
--- a/src/ports/SkOSFile_posix.cpp
+++ b/src/ports/SkOSFile_posix.cpp
@@ -95,6 +95,18 @@
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 915b87b..ee997e9 100644
--- a/src/ports/SkOSFile_stdio.cpp
+++ b/src/ports/SkOSFile_stdio.cpp
@@ -80,15 +80,6 @@
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);
@@ -107,32 +98,6 @@
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);
@@ -151,16 +116,6 @@
#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) {
@@ -170,8 +125,9 @@
}
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 6bdf9ab..414393a 100644
--- a/src/ports/SkOSFile_win.cpp
+++ b/src/ports/SkOSFile_win.cpp
@@ -124,6 +124,33 @@
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/tests/PathOpsExtendedTest.cpp b/tests/PathOpsExtendedTest.cpp
index c96cbcd..9e2dfbe 100644
--- a/tests/PathOpsExtendedTest.cpp
+++ b/tests/PathOpsExtendedTest.cpp
@@ -595,7 +595,7 @@
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 a3df8d7..031cc1a 100644
--- a/tests/StreamTest.cpp
+++ b/tests/StreamTest.cpp
@@ -67,7 +67,7 @@
{
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 f49e126..ff14b5b 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 @@
return false;
}
testdata->resize(len);
- (void) sk_fread(testdata->writable_str(), len, file);
+ (void) fread(testdata->writable_str(), len, file);
return true;
}
