Fixed bugs on manipulating small files

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)

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<uint>(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<ulong>(fileSize);
-    return d->size;
+    return static_cast<ulong>(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;
 }