From dcf11b95861a72f3c5c23ff0763ee2d74d948449 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Sun, 19 May 2013 02:33:17 +0900 Subject: [PATCH] Small refactoring of FileStream --- taglib/toolkit/tbytevector.cpp | 8 +- taglib/toolkit/tfilestream.cpp | 228 +++++++++++++++------------------ 2 files changed, 106 insertions(+), 130 deletions(-) diff --git a/taglib/toolkit/tbytevector.cpp b/taglib/toolkit/tbytevector.cpp index 471af477..b3c8897f 100644 --- a/taglib/toolkit/tbytevector.cpp +++ b/taglib/toolkit/tbytevector.cpp @@ -729,9 +729,11 @@ TagLib::uint ByteVector::size() const ByteVector &ByteVector::resize(uint size, char padding) { - detach(); - d->data->data.resize(d->offset + size, padding); - d->length = size; + if(size != d->length) { + detach(); + d->data->data.resize(d->offset + size, padding); + d->length = size; + } return *this; } diff --git a/taglib/toolkit/tfilestream.cpp b/taglib/toolkit/tfilestream.cpp index 2ce5ab41..7a931cbe 100644 --- a/taglib/toolkit/tfilestream.cpp +++ b/taglib/toolkit/tfilestream.cpp @@ -51,9 +51,10 @@ namespace typedef FileName FileNameHandle; -# define INVALID_FILE INVALID_HANDLE_VALUE + typedef HANDLE FileHandle; + const FileHandle InvalidFileHandle = INVALID_HANDLE_VALUE; - HANDLE openFile(const FileName &path, bool readOnly) + inline FileHandle openFile(const FileName &path, bool readOnly) { const DWORD access = readOnly ? GENERIC_READ : (GENERIC_READ | GENERIC_WRITE); @@ -62,29 +63,32 @@ namespace else if(!path.str().empty()) return CreateFileA(path.str().c_str(), access, FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, NULL); else - return INVALID_FILE; + return InvalidFileHandle; } - size_t fread(void *ptr, size_t size, size_t nmemb, HANDLE stream) + inline void closeFile(FileHandle file) { - DWORD readLen; - if(ReadFile(stream, ptr, size * nmemb, &readLen, NULL)) - return (readLen / size); + CloseHandle(file); + } + + inline size_t readFile(FileHandle file, ByteVector &buffer) + { + DWORD length; + if(ReadFile(file, buffer.data(), static_cast(buffer.size()), &length, NULL)) + return static_cast(length); else return 0; } - size_t fwrite(const void *ptr, size_t size, size_t nmemb, HANDLE stream) + inline size_t writeFile(FileHandle file, const ByteVector &buffer) { - DWORD writtenLen; - if(WriteFile(stream, ptr, size * nmemb, &writtenLen, NULL)) - return (writtenLen / size); + DWORD length; + if(WriteFile(file, buffer.data(), static_cast(buffer.size()), &length, NULL)) + return static_cast(length); else return 0; } -# if _DEBUG - // Convert a string in a local encoding into a UTF-16 string. // This function should only be used to generate an error message. @@ -111,78 +115,59 @@ namespace } } -# endif - #else -# define INVALID_FILE 0 - struct FileNameHandle : public std::string { FileNameHandle(FileName name) : std::string(name) {} operator FileName () const { return c_str(); } }; - FILE *openFile(const FileName &path, bool readOnly) + typedef FILE* FileHandle; + const FileHandle InvalidFileHandle = 0; + + inline FileHandle openFile(const FileName &path, bool readOnly) { return fopen(path, readOnly ? "rb" : "rb+"); } + inline void closeFile(FileHandle file) + { + fclose(file); + } + + inline size_t readFile(FileHandle file, ByteVector &buffer) + { + return fread(buffer.data(), sizeof(char), buffer.size(), file); + } + + inline size_t writeFile(FileHandle file, const ByteVector &buffer) + { + return fwrite(buffer.data(), sizeof(char), buffer.size(), file); + } + #endif } class FileStream::FileStreamPrivate { public: - FileStreamPrivate(const FileName &fileName, bool openReadOnly); - -#ifdef _WIN32 - - HANDLE file; - -#else - - FILE *file; - -#endif + FileStreamPrivate(const FileName &fileName, bool openReadOnly) + : file(InvalidFileHandle) + , name(fileName) + , readOnly(openReadOnly) + , size(0) + { + } + FileHandle file; FileNameHandle name; - bool readOnly; ulong size; + static const uint bufferSize = 1024; }; -FileStream::FileStreamPrivate::FileStreamPrivate(const FileName &fileName, bool openReadOnly) : - file(INVALID_FILE), - name(fileName), - readOnly(true), - size(0) -{ - // First try with read / write mode, if that fails, fall back to read only. - - if(!openReadOnly) - file = openFile(name, false); - - if(file != INVALID_FILE) - readOnly = false; - else - file = openFile(name, true); - - if(file == INVALID_FILE) - { -# ifdef _WIN32 - - debug("Could not open file " + fileNameToString(name)); - -# else - - debug("Could not open file " + String((const char *) name)); - -# endif - } -} - //////////////////////////////////////////////////////////////////////////////// // public members //////////////////////////////////////////////////////////////////////////////// @@ -190,21 +175,30 @@ FileStream::FileStreamPrivate::FileStreamPrivate(const FileName &fileName, bool FileStream::FileStream(FileName file, bool openReadOnly) : d(new FileStreamPrivate(file, openReadOnly)) { + // First try with read / write mode, if that fails, fall back to read only. + + if(!openReadOnly) + d->file = openFile(file, false); + + if(d->file != InvalidFileHandle) + d->readOnly = false; + else + d->file = openFile(d->name, true); + + if(d->file == InvalidFileHandle) + { +# ifdef _WIN32 + debug("Could not open file " + fileNameToString(d->name)); +# else + debug("Could not open file " + String(static_cast(d->name))); +# endif + } } FileStream::~FileStream() { -#ifdef _WIN32 - if(isOpen()) - CloseHandle(d->file); - -#else - - if(isOpen()) - fclose(d->file); - -#endif + closeFile(d->file); delete d; } @@ -224,16 +218,16 @@ ByteVector FileStream::readBlock(ulong length) if(length == 0) return ByteVector::null; - if(length > FileStreamPrivate::bufferSize && - length > ulong(FileStream::length())) - { - length = FileStream::length(); - } + const ulong streamLength = static_cast(FileStream::length()); + if(length > bufferSize() && length > streamLength) + length = streamLength; - ByteVector v(static_cast(length)); - const int count = fread(v.data(), sizeof(char), length, d->file); - v.resize(count); - return v; + ByteVector buffer(static_cast(length)); + + const size_t count = readFile(d->file, buffer); + buffer.resize(static_cast(count)); + + return buffer; } void FileStream::writeBlock(const ByteVector &data) @@ -248,7 +242,7 @@ void FileStream::writeBlock(const ByteVector &data) return; } - fwrite(data.data(), sizeof(char), data.size(), d->file); + writeFile(d->file, data); } void FileStream::insert(const ByteVector &data, ulong start, ulong replace) @@ -269,10 +263,10 @@ void FileStream::insert(const ByteVector &data, ulong start, ulong replace) return; } else if(data.size() < replace) { - seek(start); - writeBlock(data); - removeBlock(start + data.size(), replace - data.size()); - return; + seek(start); + writeBlock(data); + removeBlock(start + data.size(), replace - data.size()); + return; } // Woohoo! Faster (about 20%) than id3lib at last. I had to get hardcore @@ -295,64 +289,41 @@ void FileStream::insert(const ByteVector &data, ulong start, ulong replace) long readPosition = start + replace; long writePosition = start; - ByteVector buffer; + ByteVector buffer = data; ByteVector aboutToOverwrite(static_cast(bufferLength)); - // This is basically a special case of the loop below. Here we're just - // doing the same steps as below, but since we aren't using the same buffer - // size -- instead we're using the tag size -- this has to be handled as a - // special case. We're also using File::writeBlock() just for the tag. - // That's a bit slower than using char *'s so, we're only doing it here. - - seek(readPosition); - int bytesRead = fread(aboutToOverwrite.data(), sizeof(char), bufferLength, d->file); - readPosition += bufferLength; - - seek(writePosition); - writeBlock(data); - writePosition += data.size(); - - buffer = aboutToOverwrite; - - // In case we've already reached the end of file... - - buffer.resize(bytesRead); - - // Ok, here's the main loop. We want to loop until the read fails, which - // means that we hit the end of the file. - - while(!buffer.isEmpty()) { - + while(true) + { // Seek to the current read position and read the data that we're about // to overwrite. Appropriately increment the readPosition. - + seek(readPosition); - bytesRead = fread(aboutToOverwrite.data(), sizeof(char), bufferLength, d->file); + const size_t bytesRead = readFile(d->file, aboutToOverwrite); aboutToOverwrite.resize(bytesRead); readPosition += bufferLength; // 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(ulong(bytesRead) < bufferLength) + if(bytesRead < bufferLength) clear(); // Seek to the write position and write our buffer. Increment the // writePosition. seek(writePosition); - fwrite(buffer.data(), sizeof(char), buffer.size(), d->file); + writeBlock(buffer); + + // We hit the end of the file. + + if(bytesRead == 0) + break; + writePosition += buffer.size(); // Make the current buffer the data that we read in the beginning. - + buffer = aboutToOverwrite; - - // Again, we need this for the last write. We don't want to write garbage - // at the end of our file, so we need to set the buffer size to the amount - // that we actually read. - - bufferLength = bytesRead; } } @@ -370,11 +341,9 @@ void FileStream::removeBlock(ulong start, ulong length) ByteVector buffer(static_cast(bufferLength)); - ulong bytesRead = 1; - - while(bytesRead != 0) { + while(true) { seek(readPosition); - bytesRead = fread(buffer.data(), sizeof(char), bufferLength, d->file); + const size_t bytesRead = readFile(d->file, buffer); readPosition += bytesRead; // Check to see if we just read the last block. We need to call clear() @@ -384,9 +353,14 @@ void FileStream::removeBlock(ulong start, ulong length) clear(); seek(writePosition); - fwrite(buffer.data(), sizeof(char), bytesRead, d->file); + writeFile(d->file, buffer); + + if(bytesRead == 0) + break; + writePosition += bytesRead; } + truncate(writePosition); } @@ -397,7 +371,7 @@ bool FileStream::readOnly() const bool FileStream::isOpen() const { - return (d->file != INVALID_FILE); + return (d->file != InvalidFileHandle); } void FileStream::seek(long offset, Position p)