From 4667ba02e551bde49c090a5d0321ef5d078b5b8d Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Thu, 23 May 2013 17:38:43 +0900 Subject: [PATCH] Fixed bugs on manipulating small files --- taglib/toolkit/tfile.cpp | 38 ++++++++++++++++++++-------------- taglib/toolkit/tfilestream.cpp | 37 +++++++++++++-------------------- 2 files changed, 37 insertions(+), 38 deletions(-) diff --git a/taglib/toolkit/tfile.cpp b/taglib/toolkit/tfile.cpp index a229e85f..279524b3 100644 --- a/taglib/toolkit/tfile.cpp +++ b/taglib/toolkit/tfile.cpp @@ -66,6 +66,15 @@ using namespace TagLib; +namespace +{ +#ifdef _WIN32 + const uint BufferSize = 8192; +#else + const uint BufferSize = 1024; +#endif +} + class File::FilePrivate { public: @@ -74,7 +83,6 @@ public: IOStream *stream; bool streamOwner; bool valid; - static const uint bufferSize = 1024; }; File::FilePrivate::FilePrivate(IOStream *stream, bool owner) : @@ -237,7 +245,7 @@ void File::writeBlock(const ByteVector &data) long File::find(const ByteVector &pattern, long fromOffset, const ByteVector &before) { - if(!d->stream || pattern.size() > d->bufferSize) + if(!d->stream || pattern.size() > bufferSize()) return -1; // The position in the file that the current buffer starts at. @@ -278,20 +286,20 @@ long File::find(const ByteVector &pattern, long fromOffset, const ByteVector &be // then check for "before". The order is important because it gives priority // to "real" matches. - for(buffer = readBlock(d->bufferSize); buffer.size() > 0; buffer = readBlock(d->bufferSize)) { + for(buffer = readBlock(bufferSize()); buffer.size() > 0; buffer = readBlock(bufferSize())) { // (1) previous partial match - if(previousPartialMatch >= 0 && int(d->bufferSize) > previousPartialMatch) { - const int patternOffset = (d->bufferSize - previousPartialMatch); + if(previousPartialMatch >= 0 && int(bufferSize()) > previousPartialMatch) { + const int patternOffset = (bufferSize() - previousPartialMatch); if(buffer.containsAt(pattern, 0, patternOffset)) { seek(originalPosition); - return bufferOffset - d->bufferSize + previousPartialMatch; + return bufferOffset - bufferSize() + previousPartialMatch; } } - if(!before.isNull() && beforePreviousPartialMatch >= 0 && int(d->bufferSize) > beforePreviousPartialMatch) { - const int beforeOffset = (d->bufferSize - beforePreviousPartialMatch); + if(!before.isNull() && beforePreviousPartialMatch >= 0 && int(bufferSize()) > beforePreviousPartialMatch) { + const int beforeOffset = (bufferSize() - beforePreviousPartialMatch); if(buffer.containsAt(before, 0, beforeOffset)) { seek(originalPosition); return -1; @@ -318,7 +326,7 @@ long File::find(const ByteVector &pattern, long fromOffset, const ByteVector &be if(!before.isNull()) beforePreviousPartialMatch = buffer.endsWithPartialMatch(before); - bufferOffset += d->bufferSize; + bufferOffset += bufferSize(); } // Since we hit the end of the file, reset the status before continuing. @@ -333,7 +341,7 @@ long File::find(const ByteVector &pattern, long fromOffset, const ByteVector &be long File::rfind(const ByteVector &pattern, long fromOffset, const ByteVector &before) { - if(!d->stream || pattern.size() > d->bufferSize) + if(!d->stream || pattern.size() > bufferSize()) return -1; // The position in the file that the current buffer starts at. @@ -357,17 +365,17 @@ long File::rfind(const ByteVector &pattern, long fromOffset, const ByteVector &b long bufferOffset; if(fromOffset == 0) { - seek(-1 * int(d->bufferSize), End); + seek(-1 * int(bufferSize()), End); bufferOffset = tell(); } else { - seek(fromOffset + -1 * int(d->bufferSize), Beginning); + seek(fromOffset + -1 * int(bufferSize()), Beginning); bufferOffset = tell(); } // See the notes in find() for an explanation of this algorithm. - for(buffer = readBlock(d->bufferSize); buffer.size() > 0; buffer = readBlock(d->bufferSize)) { + for(buffer = readBlock(bufferSize()); buffer.size() > 0; buffer = readBlock(bufferSize())) { // TODO: (1) previous partial match @@ -386,7 +394,7 @@ long File::rfind(const ByteVector &pattern, long fromOffset, const ByteVector &b // TODO: (3) partial match - bufferOffset -= d->bufferSize; + bufferOffset -= bufferSize(); seek(bufferOffset); } @@ -485,7 +493,7 @@ bool File::isWritable(const char *file) TagLib::uint File::bufferSize() { - return FilePrivate::bufferSize; + return BufferSize; } void File::setValid(bool valid) diff --git a/taglib/toolkit/tfilestream.cpp b/taglib/toolkit/tfilestream.cpp index 1798134f..9cd1be42 100644 --- a/taglib/toolkit/tfilestream.cpp +++ b/taglib/toolkit/tfilestream.cpp @@ -40,11 +40,12 @@ namespace { #ifdef _WIN32 - // Using Win32 native API instead of standard C file I/O to reduce the resource consumption. + // Uses Win32 native API instead of POSIX API to reduce the resource consumption. typedef FileName FileNameHandle; - typedef HANDLE FileHandle; + + const uint BufferSize = 8192; const FileHandle InvalidFileHandle = INVALID_HANDLE_VALUE; inline FileHandle openFile(const FileName &path, bool readOnly) @@ -120,6 +121,8 @@ namespace }; typedef FILE* FileHandle; + + const uint BufferSize = 8192; const FileHandle InvalidFileHandle = 0; inline FileHandle openFile(const FileName &path, bool readOnly) @@ -152,16 +155,12 @@ public: : file(InvalidFileHandle) , name(fileName) , readOnly(true) - , size(0) { } FileHandle file; FileNameHandle name; bool readOnly; - ulong size; - - static const uint bufferSize = 1024; }; //////////////////////////////////////////////////////////////////////////////// @@ -337,23 +336,23 @@ void FileStream::removeBlock(ulong start, ulong length) ByteVector buffer(static_cast(bufferLength)); - while(true) { + for(size_t bytesRead = -1; bytesRead != 0;) + { seek(readPosition); - const size_t bytesRead = readFile(d->file, buffer); + bytesRead = readFile(d->file, buffer); readPosition += bytesRead; // Check to see if we just read the last block. We need to call clear() // if we did so that the last write succeeds. - if(bytesRead < bufferLength) + if(bytesRead < buffer.size()) { clear(); + buffer.resize(bytesRead); + } seek(writePosition); writeFile(d->file, buffer); - if(bytesRead == 0) - break; - writePosition += bytesRead; } @@ -397,7 +396,7 @@ void FileStream::seek(long offset, Position p) SetFilePointer(d->file, offset, NULL, whence); if(GetLastError() != NO_ERROR) { - debug("File::seek() -- Failed to set the file size."); + debug("File::seek() -- Failed to set the file pointer."); } #else @@ -463,21 +462,14 @@ long FileStream::length() return 0; } - // Do some caching in case we do multiple calls. - - if(d->size > 0) - return d->size; - #ifdef _WIN32 const DWORD fileSize = GetFileSize(d->file, NULL); if(GetLastError() == NO_ERROR) { - d->size = static_cast(fileSize); - return d->size; + return static_cast(fileSize); } else { debug("File::length() -- Failed to get the file size."); - d->size = 0; return 0; } @@ -490,7 +482,6 @@ long FileStream::length() seek(curpos, Beginning); - d->size = endpos; return endpos; #endif @@ -526,5 +517,5 @@ void FileStream::truncate(long length) TagLib::uint FileStream::bufferSize() { - return FileStreamPrivate::bufferSize; + return BufferSize; }