Fix File::rfind() for small files to work just like ByteVector::rfind().

diff --git a/taglib/toolkit/tfile.cpp b/taglib/toolkit/tfile.cpp
index 4a05b05f0a1a164b8b4018e29f77a844d5cd4c90..af8ec3901b64e2f69f6320b0d1e68c640e1731ef 100644 (file)
--- a/taglib/toolkit/tfile.cpp
+++ b/taglib/toolkit/tfile.cpp
@@ -278,7 +278,7 @@ long File::find(const ByteVector &pattern, long fromOffset, const ByteVector &be
   // (2) The search pattern is wholly contained within the current buffer.
   //
   // (3) The current buffer ends with a partial match of the pattern.  We will
-  // note this for use in the next itteration, where we will check for the rest
+  // note this for use in the next iteration, where we will check for the rest
   // of the pattern.
   //
   // All three of these are done in two steps.  First we check for the pattern
@@ -363,25 +363,34 @@ long File::rfind(const ByteVector &pattern, long fromOffset, const ByteVector &b
 
   // Start the search at the offset.
 
-  long bufferOffset;
-  if(fromOffset == 0) {
-    seek(-1 * int(bufferSize()), End);
-    bufferOffset = tell();
-  }
-  else {
-    seek(fromOffset + -1 * int(bufferSize()), Beginning);
-    bufferOffset = tell();
-  }
+  if(fromOffset == 0)
+    fromOffset = length();
+
+  long bufferLength = bufferSize();
+  long bufferOffset = fromOffset + pattern.size();
 
   // See the notes in find() for an explanation of this algorithm.
 
-  for(buffer = readBlock(bufferSize()); buffer.size() > 0; buffer = readBlock(bufferSize())) {
+  while(true) {
+
+    if(bufferOffset > bufferLength) {
+      bufferOffset -= bufferLength;
+    }
+    else {
+      bufferLength = bufferOffset;
+      bufferOffset = 0;
+    }
+    seek(bufferOffset);
+
+    buffer = readBlock(bufferLength);
+    if(buffer.isEmpty())
+      break;
 
     // TODO: (1) previous partial match
 
     // (2) pattern contained in current buffer
 
-    long location = buffer.rfind(pattern);
+    const long location = buffer.rfind(pattern);
     if(location >= 0) {
       seek(originalPosition);
       return bufferOffset + location;
@@ -393,9 +402,6 @@ long File::rfind(const ByteVector &pattern, long fromOffset, const ByteVector &b
     }
 
     // TODO: (3) partial match
-
-    bufferOffset -= bufferSize();
-    seek(bufferOffset);
   }
 
   // Since we hit the end of the file, reset the status before continuing.

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 7a3d0bd51029a82a54b4ec4ecd273d819450649c..890013b0fe86b98fe27a74533c521c655f9aa1fc 100644 (file)
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -38,6 +38,7 @@ SET(test_runner_SRCS
   test_bytevectorstream.cpp
   test_string.cpp
   test_propertymap.cpp
+  test_file.cpp
   test_fileref.cpp
   test_id3v1.cpp
   test_id3v2.cpp

diff --git a/tests/test_file.cpp b/tests/test_file.cpp
new file mode 100644 (file)
index 0000000..b3751a2
--- /dev/null
+++ b/tests/test_file.cpp
@@ -0,0 +1,105 @@
+/***************************************************************************
+    copyright           : (C) 2014 by Lukas Lalinsky
+    email               : lukas@oxygene.sk
+ ***************************************************************************/
+
+/***************************************************************************
+ *   This library is free software; you can redistribute it and/or modify  *
+ *   it  under the terms of the GNU Lesser General Public License version  *
+ *   2.1 as published by the Free Software Foundation.                     *
+ *                                                                         *
+ *   This library is distributed in the hope that it will be useful, but   *
+ *   WITHOUT ANY WARRANTY; without even the implied warranty of            *
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU     *
+ *   Lesser General Public License for more details.                       *
+ *                                                                         *
+ *   You should have received a copy of the GNU Lesser General Public      *
+ *   License along with this library; if not, write to the Free Software   *
+ *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,            *
+ *   MA  02110-1301  USA                                                   *
+ ***************************************************************************/
+
+#include <tfile.h>
+#include <cppunit/extensions/HelperMacros.h>
+#include "utils.h"
+
+using namespace TagLib;
+
+// File subclass that gives tests access to filesystem operations
+class PlainFile : public File {
+public:
+  PlainFile(FileName name) : File(name) { }
+  Tag *tag() const { return NULL; }
+  AudioProperties *audioProperties() const { return NULL; }
+  bool save(){ return false; }
+  void truncate(long length) { File::truncate(length); }
+};
+
+class TestFile : public CppUnit::TestFixture
+{
+  CPPUNIT_TEST_SUITE(TestFile);
+  CPPUNIT_TEST(testFindInSmallFile);
+  CPPUNIT_TEST(testRFindInSmallFile);
+  CPPUNIT_TEST_SUITE_END();
+
+public:
+
+  void testFindInSmallFile()
+  {
+    ScopedFileCopy copy("empty", ".ogg");
+    std::string name = copy.fileName();
+    {
+      PlainFile file(name.c_str());
+      file.seek(0);
+      file.writeBlock(ByteVector("0123456239", 10));
+      file.truncate(10);
+    }
+    {
+      PlainFile file(name.c_str());
+      CPPUNIT_ASSERT_EQUAL(10l, file.length());
+
+      CPPUNIT_ASSERT_EQUAL(2l, file.find(ByteVector("23", 2)));
+      CPPUNIT_ASSERT_EQUAL(2l, file.find(ByteVector("23", 2), 2));
+      CPPUNIT_ASSERT_EQUAL(7l, file.find(ByteVector("23", 2), 3));
+
+      file.seek(0);
+      const ByteVector v = file.readBlock(file.length());
+      CPPUNIT_ASSERT_EQUAL((uint)10, v.size());
+
+      CPPUNIT_ASSERT_EQUAL((long)v.find("23"),    file.find("23"));
+      CPPUNIT_ASSERT_EQUAL((long)v.find("23", 2), file.find("23", 2));
+      CPPUNIT_ASSERT_EQUAL((long)v.find("23", 3), file.find("23", 3));
+    }
+  }
+
+  void testRFindInSmallFile()
+  {
+    ScopedFileCopy copy("empty", ".ogg");
+    std::string name = copy.fileName();
+    {
+      PlainFile file(name.c_str());
+      file.seek(0);
+      file.writeBlock(ByteVector("0123456239", 10));
+      file.truncate(10);
+    }
+    {
+      PlainFile file(name.c_str());
+      CPPUNIT_ASSERT_EQUAL(10l, file.length());
+
+      CPPUNIT_ASSERT_EQUAL(7l, file.rfind(ByteVector("23", 2)));
+      CPPUNIT_ASSERT_EQUAL(7l, file.rfind(ByteVector("23", 2), 7));
+      CPPUNIT_ASSERT_EQUAL(2l, file.rfind(ByteVector("23", 2), 6));
+
+      file.seek(0);
+      const ByteVector v = file.readBlock(file.length());
+      CPPUNIT_ASSERT_EQUAL((uint)10, v.size());
+
+      CPPUNIT_ASSERT_EQUAL((long)v.rfind("23"),    file.rfind("23"));
+      CPPUNIT_ASSERT_EQUAL((long)v.rfind("23", 7), file.rfind("23", 7));
+      CPPUNIT_ASSERT_EQUAL((long)v.rfind("23", 6), file.rfind("23", 6));
+    }
+  }
+
+};
+
+CPPUNIT_TEST_SUITE_REGISTRATION(TestFile);