From b6361ddde538bebc7a46b231543eebafb01ae0ed Mon Sep 17 00:00:00 2001
From: Tsuda Kageyu <tsuda.kageyu@gmail.com>
Date: Wed, 25 Nov 2015 01:14:31 +0900
Subject: [PATCH 01/19] Fix saving MPEG files.

This fixes all the issues reported at #618.
---
 taglib/mpeg/mpegfile.cpp | 237 +++++++++++++++------------------------
 taglib/mpeg/mpegfile.h   |  17 ---
 tests/test_id3v2.cpp     |  31 ++---
 tests/test_mpeg.cpp      | 120 ++++++++++++++++++++
 4 files changed, 221 insertions(+), 184 deletions(-)

diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp
index 523f804c..6fbbf19e 100644
--- a/taglib/mpeg/mpegfile.cpp
+++ b/taglib/mpeg/mpegfile.cpp
@@ -24,6 +24,7 @@
  ***************************************************************************/
 
 #include <tagunion.h>
+#include <tagutils.h>
 #include <id3v2tag.h>
 #include <id3v2header.h>
 #include <id3v1tag.h>
@@ -69,15 +70,9 @@ public:
     ID3v2Location(-1),
     ID3v2OriginalSize(0),
     APELocation(-1),
-    APEFooterLocation(-1),
     APEOriginalSize(0),
     ID3v1Location(-1),
-    hasID3v2(false),
-    hasID3v1(false),
-    hasAPE(false),
-    properties(0)
-  {
-  }
+    properties(0) {}
 
   ~FilePrivate()
   {
@@ -97,13 +92,6 @@ public:
 
   TagUnion tag;
 
-  // These indicate whether the file *on disk* has these tags, not if
-  // this data structure does.  This is used in computing offsets.
-
-  bool hasID3v2;
-  bool hasID3v1;
-  bool hasAPE;
-
   Properties *properties;
 };
 
@@ -194,17 +182,6 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version)
 
 bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version, bool duplicateTags)
 {
-  if(tags == NoTags && stripOthers)
-    return strip(AllTags);
-
-  if(!ID3v2Tag() && !ID3v1Tag() && !APETag()) {
-
-    if((d->hasID3v1 || d->hasID3v2 || d->hasAPE) && stripOthers)
-      return strip(AllTags);
-
-    return true;
-  }
-
   if(readOnly()) {
     debug("MPEG::File::save() -- File is read only.");
     return false;
@@ -212,7 +189,7 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version, bool duplica
 
   // Create the tags if we've been asked to.
 
-  if (duplicateTags) {
+  if(duplicateTags) {
 
     // Copy the values from the tag that does exist into the new tag,
     // except if the existing tag is to be stripped.
@@ -224,79 +201,93 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version, bool duplica
       Tag::duplicate(ID3v2Tag(), ID3v1Tag(true), false);
   }
 
-  bool success = true;
+  // Remove all the tags not going to be saved.
+
+  if(stripOthers)
+    strip(~tags, false);
 
   if(ID3v2 & tags) {
 
     if(ID3v2Tag() && !ID3v2Tag()->isEmpty()) {
 
-      if(!d->hasID3v2)
+      // ID3v2 tag is not empty. Update the old one or create a new one.
+
+      if(d->ID3v2Location < 0)
         d->ID3v2Location = 0;
 
-      insert(ID3v2Tag()->render(id3v2Version), d->ID3v2Location, d->ID3v2OriginalSize);
+      const ByteVector data = ID3v2Tag()->render(id3v2Version);
+      insert(data, d->ID3v2Location, d->ID3v2OriginalSize);
 
-      d->hasID3v2 = true;
+      if(d->APELocation >= 0)
+        d->APELocation += (data.size() - d->ID3v2OriginalSize);
 
-      // v1 tag location has changed, update if it exists
+      if(d->ID3v1Location >= 0)
+        d->ID3v1Location += (data.size() - d->ID3v2OriginalSize);
 
-      if(ID3v1Tag())
-        d->ID3v1Location = findID3v1();
-
-      // APE tag location has changed, update if it exists
-
-      if(APETag())
-        findAPE();
+      d->ID3v2OriginalSize = data.size();
+    }
+    else {
+
+      // ID3v2 tag is empty. Remove the old one.
+
+      strip(ID3v2, false);
     }
-    else if(stripOthers)
-      success = strip(ID3v2, false) && success;
   }
-  else if(d->hasID3v2 && stripOthers)
-    success = strip(ID3v2) && success;
 
   if(ID3v1 & tags) {
+
     if(ID3v1Tag() && !ID3v1Tag()->isEmpty()) {
-      int offset = d->hasID3v1 ? -128 : 0;
-      seek(offset, End);
-      writeBlock(ID3v1Tag()->render());
-      d->hasID3v1 = true;
-      d->ID3v1Location = findID3v1();
-    }
-    else if(stripOthers)
-      success = strip(ID3v1) && success;
-  }
-  else if(d->hasID3v1 && stripOthers)
-    success = strip(ID3v1, false) && success;
 
-  // Dont save an APE-tag unless one has been created
+      // ID3v1 tag is not empty. Update the old one or create a new one.
 
-  if((APE & tags) && APETag()) {
-    if(d->hasAPE)
-      insert(APETag()->render(), d->APELocation, d->APEOriginalSize);
-    else {
-      if(d->hasID3v1) {
-        insert(APETag()->render(), d->ID3v1Location, 0);
-        d->APEOriginalSize = APETag()->footer()->completeTagSize();
-        d->hasAPE = true;
-        d->APELocation = d->ID3v1Location;
-        d->ID3v1Location += d->APEOriginalSize;
+      if(d->ID3v1Location >= 0) {
+        seek(d->ID3v1Location);
       }
       else {
         seek(0, End);
-        d->APELocation = tell();
-        APE::Tag *apeTag = d->tag.access<APE::Tag>(APEIndex, false);
-        d->APEFooterLocation = d->APELocation
-                               + apeTag->footer()->completeTagSize()
-                               - APE::Footer::size();
-        writeBlock(APETag()->render());
-        d->APEOriginalSize = APETag()->footer()->completeTagSize();
-        d->hasAPE = true;
+        d->ID3v1Location = tell();
       }
+
+      writeBlock(ID3v1Tag()->render());
+    }
+    else {
+
+      // ID3v1 tag is empty. Remove the old one.
+
+      strip(ID3v1, false);
     }
   }
-  else if(d->hasAPE && stripOthers)
-    success = strip(APE, false) && success;
 
-  return success;
+  if(APE & tags) {
+
+    if(APETag() && !APETag()->isEmpty()) {
+
+      // APE tag is not empty. Update the old one or create a new one.
+
+      if(d->APELocation < 0) {
+        if(d->ID3v1Location >= 0)
+          d->APELocation = d->ID3v1Location;
+        else
+          d->APELocation = length();
+      }
+
+      const ByteVector data = APETag()->render();
+      insert(data, d->APELocation, d->APEOriginalSize);
+
+      if(d->ID3v1Location >= 0)
+        d->ID3v1Location += (data.size() - d->APEOriginalSize);
+
+      d->APEOriginalSize = data.size();
+    }
+    else {
+
+      // APE tag is empty. Remove the old one.
+
+      strip(APE, false);
+    }
+  }
+
+  return true;
 }
 
 ID3v2::Tag *MPEG::File::ID3v2Tag(bool create)
@@ -326,44 +317,39 @@ bool MPEG::File::strip(int tags, bool freeMemory)
     return false;
   }
 
-  if((tags & ID3v2) && d->hasID3v2) {
+  if((tags & ID3v2) && d->ID3v2Location >= 0) {
     removeBlock(d->ID3v2Location, d->ID3v2OriginalSize);
+
+    if(d->APELocation >= 0)
+      d->APELocation -= d->ID3v2OriginalSize;
+
+    if(d->ID3v1Location >= 0)
+      d->ID3v1Location -= d->ID3v2OriginalSize;
+
     d->ID3v2Location = -1;
     d->ID3v2OriginalSize = 0;
-    d->hasID3v2 = false;
 
     if(freeMemory)
       d->tag.set(ID3v2Index, 0);
-
-    // v1 tag location has changed, update if it exists
-
-    if(ID3v1Tag())
-      d->ID3v1Location = findID3v1();
-
-    // APE tag location has changed, update if it exists
-
-   if(APETag())
-      findAPE();
   }
 
-  if((tags & ID3v1) && d->hasID3v1) {
+  if((tags & ID3v1) && d->ID3v1Location >= 0) {
     truncate(d->ID3v1Location);
+
     d->ID3v1Location = -1;
-    d->hasID3v1 = false;
 
     if(freeMemory)
       d->tag.set(ID3v1Index, 0);
   }
 
-  if((tags & APE) && d->hasAPE) {
+  if((tags & APE) && d->APELocation >= 0) {
     removeBlock(d->APELocation, d->APEOriginalSize);
+
+    if(d->ID3v1Location >= 0)
+      d->ID3v1Location -= d->APEOriginalSize;
+
     d->APELocation = -1;
-    d->APEFooterLocation = -1;
-    d->hasAPE = false;
-    if(d->hasID3v1) {
-      if(d->ID3v1Location > d->APELocation)
-        d->ID3v1Location -= d->APEOriginalSize;
-    }
+    d->APEOriginalSize = 0;
 
     if(freeMemory)
       d->tag.set(APEIndex, 0);
@@ -457,17 +443,17 @@ long MPEG::File::lastFrameOffset()
 
 bool MPEG::File::hasID3v1Tag() const
 {
-  return d->hasID3v1;
+  return (d->ID3v1Location >= 0);
 }
 
 bool MPEG::File::hasID3v2Tag() const
 {
-  return d->hasID3v2;
+  return (d->ID3v2Location >= 0);
 }
 
 bool MPEG::File::hasAPETag() const
 {
-  return d->hasAPE;
+  return (d->APELocation >= 0);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -481,35 +467,25 @@ void MPEG::File::read(bool readProperties)
   d->ID3v2Location = findID3v2();
 
   if(d->ID3v2Location >= 0) {
-
     d->tag.set(ID3v2Index, new ID3v2::Tag(this, d->ID3v2Location, d->ID3v2FrameFactory));
-
     d->ID3v2OriginalSize = ID3v2Tag()->header()->completeTagSize();
-
-    if(ID3v2Tag()->header()->tagSize() <= 0)
-      d->tag.set(ID3v2Index, 0);
-    else
-      d->hasID3v2 = true;
   }
 
   // Look for an ID3v1 tag
 
-  d->ID3v1Location = findID3v1();
+  d->ID3v1Location = Utils::findID3v1(this);
 
-  if(d->ID3v1Location >= 0) {
+  if(d->ID3v1Location >= 0)
     d->tag.set(ID3v1Index, new ID3v1::Tag(this, d->ID3v1Location));
-    d->hasID3v1 = true;
-  }
 
   // Look for an APE tag
 
-  findAPE();
+  d->APELocation = Utils::findAPE(this, d->ID3v1Location);
 
   if(d->APELocation >= 0) {
-
-    d->tag.set(APEIndex, new APE::Tag(this, d->APEFooterLocation));
+    d->tag.set(APEIndex, new APE::Tag(this, d->APELocation));
     d->APEOriginalSize = APETag()->footer()->completeTagSize();
-    d->hasAPE = true;
+    d->APELocation = d->APELocation + APE::Footer::size() - d->APEOriginalSize;
   }
 
   if(readProperties)
@@ -556,36 +532,3 @@ long MPEG::File::findID3v2()
 
   return tagOffset;
 }
-
-long MPEG::File::findID3v1()
-{
-  if(isValid()) {
-    seek(-128, End);
-    long p = tell();
-
-    if(readBlock(3) == ID3v1::Tag::fileIdentifier())
-      return p;
-  }
-  return -1;
-}
-
-void MPEG::File::findAPE()
-{
-  if(isValid()) {
-    seek(d->hasID3v1 ? -160 : -32, End);
-
-    long p = tell();
-
-    if(readBlock(8) == APE::Tag::fileIdentifier()) {
-      d->APEFooterLocation = p;
-      seek(d->APEFooterLocation);
-      APE::Footer footer(readBlock(APE::Footer::size()));
-      d->APELocation = d->APEFooterLocation - footer.completeTagSize()
-                       + APE::Footer::size();
-      return;
-    }
-  }
-
-  d->APELocation = -1;
-  d->APEFooterLocation = -1;
-}
diff --git a/taglib/mpeg/mpegfile.h b/taglib/mpeg/mpegfile.h
index 8478d093..e9e97387 100644
--- a/taglib/mpeg/mpegfile.h
+++ b/taglib/mpeg/mpegfile.h
@@ -175,9 +175,6 @@ namespace TagLib {
        * If you would like more granular control over the content of the tags,
        * with the concession of generality, use parameterized save call below.
        *
-       * \warning In the current implementation, it's dangerous to call save()
-       * repeatedly.  At worst it will corrupt the file.
-       *
        * \see save(int tags)
        */
       virtual bool save();
@@ -190,9 +187,6 @@ namespace TagLib {
        * This strips all tags not included in the mask, but does not modify them
        * in memory, so later calls to save() which make use of these tags will
        * remain valid.  This also strips empty tags.
-       *
-       * \warning In the current implementation, it's dangerous to call save()
-       * repeatedly.  At worst it will corrupt the file.
        */
       bool save(int tags);
 
@@ -204,9 +198,6 @@ namespace TagLib {
        * If \a stripOthers is true this strips all tags not included in the mask,
        * but does not modify them in memory, so later calls to save() which make
        * use of these tags will remain valid.  This also strips empty tags.
-       *
-       * \warning In the current implementation, it's dangerous to call save()
-       * repeatedly.  At worst it will corrupt the file.
        */
       // BIC: combine with the above method
       bool save(int tags, bool stripOthers);
@@ -222,9 +213,6 @@ namespace TagLib {
        *
        * The \a id3v2Version parameter specifies the version of the saved
        * ID3v2 tag. It can be either 4 or 3.
-       *
-       * \warning In the current implementation, it's dangerous to call save()
-       * repeatedly.  At worst it will corrupt the file.
        */
       // BIC: combine with the above method
       bool save(int tags, bool stripOthers, int id3v2Version);
@@ -243,9 +231,6 @@ namespace TagLib {
        *
        * If \a duplicateTags is true and at least one tag -- ID3v1 or ID3v2 --
        * exists this will duplicate its content into the other tag.
-       *
-       * \warning In the current implementation, it's dangerous to call save()
-       * repeatedly.  At worst it will corrupt the file.
        */
       // BIC: combine with the above method
       bool save(int tags, bool stripOthers, int id3v2Version, bool duplicateTags);
@@ -391,8 +376,6 @@ namespace TagLib {
 
       void read(bool readProperties);
       long findID3v2();
-      long findID3v1();
-      void findAPE();
 
       class FilePrivate;
       FilePrivate *d;
diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp
index c6592bbe..eececebc 100644
--- a/tests/test_id3v2.cpp
+++ b/tests/test_id3v2.cpp
@@ -1062,29 +1062,20 @@ public:
 
     {
       MPEG::File f(newname.c_str());
-      ID3v2::Tag *tag = f.ID3v2Tag(true);
-
-      ID3v2::TextIdentificationFrame *frame1 = new ID3v2::TextIdentificationFrame("TIT2");
-      frame1->setText("Title");
-      tag->addFrame(frame1);
-
-      ID3v2::AttachedPictureFrame *frame2 = new ID3v2::AttachedPictureFrame();
-      frame2->setPicture(ByteVector(100 * 1024, '\xff'));
-      tag->addFrame(frame2);
-
-      f.save();
-      CPPUNIT_ASSERT(f.length() > 100 * 1024);
+      f.ID3v2Tag()->setTitle(std::wstring(64 * 1024, L'X').c_str());
+      f.save(MPEG::File::ID3v2, true);
     }
-
     {
       MPEG::File f(newname.c_str());
-      CPPUNIT_ASSERT_EQUAL(true, f.hasID3v2Tag());
-
-      ID3v2::Tag *tag = f.ID3v2Tag();
-      tag->removeFrames("APIC");
-
-      f.save();
-      CPPUNIT_ASSERT(f.length() < 10 * 1024);
+      CPPUNIT_ASSERT(f.hasID3v2Tag());
+      CPPUNIT_ASSERT_EQUAL(74789L, f.length());
+      f.ID3v2Tag()->setTitle("ABCDEFGHIJ");
+      f.save(MPEG::File::ID3v2, true);
+    }
+    {
+      MPEG::File f(newname.c_str());
+      CPPUNIT_ASSERT(f.hasID3v2Tag());
+      CPPUNIT_ASSERT_EQUAL(9263L, f.length());
     }
   }
 
diff --git a/tests/test_mpeg.cpp b/tests/test_mpeg.cpp
index ee3575e3..b8eaa1f7 100644
--- a/tests/test_mpeg.cpp
+++ b/tests/test_mpeg.cpp
@@ -30,6 +30,12 @@ class TestMPEG : public CppUnit::TestFixture
   CPPUNIT_TEST(testFuzzedFile);
   CPPUNIT_TEST(testFrameOffset);
   CPPUNIT_TEST(testStripAndProperties);
+  CPPUNIT_TEST(testRepeatedSave1);
+  CPPUNIT_TEST(testRepeatedSave2);
+  CPPUNIT_TEST(testRepeatedSave3);
+  CPPUNIT_TEST(testEmptyID3v2);
+  CPPUNIT_TEST(testEmptyID3v1);
+  CPPUNIT_TEST(testEmptyAPE);
   CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -239,6 +245,120 @@ public:
     }
   }
 
+  void testRepeatedSave1()
+  {
+    ScopedFileCopy copy("xing", ".mp3");
+
+    {
+      MPEG::File f(copy.fileName().c_str());
+      f.ID3v2Tag(true)->setTitle(std::string(4096, 'X').c_str());
+      f.save();
+    }
+    {
+      MPEG::File f(copy.fileName().c_str());
+      f.ID3v2Tag(true)->setTitle("");
+      f.save();
+      f.ID3v2Tag(true)->setTitle(std::string(4096, 'X').c_str());
+      f.save();
+      CPPUNIT_ASSERT_EQUAL(5141L, f.firstFrameOffset());
+    }
+  }
+
+  void testRepeatedSave2()
+  {
+    ScopedFileCopy copy("xing", ".mp3");
+
+    MPEG::File f(copy.fileName().c_str());
+    f.ID3v2Tag(true)->setTitle("0123456789");
+    f.save();
+    f.save();
+    CPPUNIT_ASSERT_EQUAL(-1L, f.find("ID3", 3));
+  }
+
+  void testRepeatedSave3()
+  {
+    ScopedFileCopy copy("xing", ".mp3");
+
+    {
+      MPEG::File f(copy.fileName().c_str());
+      CPPUNIT_ASSERT(!f.hasAPETag());
+      CPPUNIT_ASSERT(!f.hasID3v1Tag());
+
+      f.APETag(true)->setTitle("01234 56789 ABCDE FGHIJ");
+      f.save();
+      f.APETag()->setTitle("0");
+      f.save();
+      f.ID3v1Tag(true)->setTitle("01234 56789 ABCDE FGHIJ");
+      f.APETag()->setTitle("01234 56789 ABCDE FGHIJ 01234 56789 ABCDE FGHIJ 01234 56789");
+      f.save();
+    }
+    {
+      MPEG::File f(copy.fileName().c_str());
+      CPPUNIT_ASSERT(f.hasAPETag());
+      CPPUNIT_ASSERT(f.hasID3v1Tag());
+    }
+  }
+
+  void testEmptyID3v2()
+  {
+    ScopedFileCopy copy("xing", ".mp3");
+
+    {
+      MPEG::File f(copy.fileName().c_str());
+      f.ID3v2Tag(true)->setTitle("0123456789");
+      f.save(MPEG::File::ID3v2);
+    }
+    {
+      MPEG::File f(copy.fileName().c_str());
+      f.ID3v2Tag(true)->setTitle("");
+      f.save(MPEG::File::ID3v2, false);
+    }
+    {
+      MPEG::File f(copy.fileName().c_str());
+      CPPUNIT_ASSERT(!f.hasID3v2Tag());
+    }
+  }
+
+  void testEmptyID3v1()
+  {
+    ScopedFileCopy copy("xing", ".mp3");
+
+    {
+      MPEG::File f(copy.fileName().c_str());
+      f.ID3v1Tag(true)->setTitle("0123456789");
+      f.save(MPEG::File::ID3v1);
+    }
+    {
+      MPEG::File f(copy.fileName().c_str());
+      f.ID3v1Tag(true)->setTitle("");
+      f.save(MPEG::File::ID3v1, false);
+    }
+    {
+      MPEG::File f(copy.fileName().c_str());
+      CPPUNIT_ASSERT(!f.hasID3v1Tag());
+    }
+  }
+
+  void testEmptyAPE()
+  {
+    ScopedFileCopy copy("xing", ".mp3");
+
+    {
+      MPEG::File f(copy.fileName().c_str());
+      f.APETag(true)->setTitle("0123456789");
+      f.save(MPEG::File::APE);
+    }
+    {
+      MPEG::File f(copy.fileName().c_str());
+      f.APETag(true)->setTitle("");
+      f.save(MPEG::File::APE, false);
+    }
+    {
+      MPEG::File f(copy.fileName().c_str());
+      CPPUNIT_ASSERT(!f.hasAPETag());
+    }
+  }
+
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(TestMPEG);

From f180445be510e95f22067dd5cd2411508d3658b1 Mon Sep 17 00:00:00 2001
From: Tsuda Kageyu <tsuda.kageyu@gmail.com>
Date: Fri, 27 Nov 2015 00:12:24 +0900
Subject: [PATCH 02/19] Fix saving APE files.

This fixes the issue reported at #624.
---
 taglib/ape/apefile.cpp | 118 ++++++++++++++++++-----------------------
 taglib/ape/apefile.h   |   3 --
 tests/test_ape.cpp     |  27 ++++++++++
 3 files changed, 79 insertions(+), 69 deletions(-)

diff --git a/taglib/ape/apefile.cpp b/taglib/ape/apefile.cpp
index 24171145..17d97668 100644
--- a/taglib/ape/apefile.cpp
+++ b/taglib/ape/apefile.cpp
@@ -61,10 +61,7 @@ public:
     ID3v2Header(0),
     ID3v2Location(-1),
     ID3v2Size(0),
-    properties(0),
-    hasAPE(false),
-    hasID3v1(false),
-    hasID3v2(false) {}
+    properties(0) {}
 
   ~FilePrivate()
   {
@@ -84,13 +81,6 @@ public:
   TagUnion tag;
 
   Properties *properties;
-
-  // These indicate whether the file *on disk* has these tags, not if
-  // this data structure does.  This is used in computing offsets.
-
-  bool hasAPE;
-  bool hasID3v1;
-  bool hasID3v2;
 };
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -155,64 +145,67 @@ bool APE::File::save()
 
   // Update ID3v1 tag
 
-  if(ID3v1Tag()) {
-    if(d->hasID3v1) {
+  if(ID3v1Tag() && !ID3v1Tag()->isEmpty()) {
+
+    // ID3v1 tag is not empty. Update the old one or create a new one.
+
+    if(d->ID3v1Location >= 0) {
       seek(d->ID3v1Location);
-      writeBlock(ID3v1Tag()->render());
     }
     else {
       seek(0, End);
       d->ID3v1Location = tell();
-      writeBlock(ID3v1Tag()->render());
-      d->hasID3v1 = true;
     }
+
+    writeBlock(ID3v1Tag()->render());
   }
   else {
-    if(d->hasID3v1) {
-      removeBlock(d->ID3v1Location, 128);
-      d->hasID3v1 = false;
-      if(d->hasAPE) {
-        if(d->APELocation > d->ID3v1Location)
-          d->APELocation -= 128;
-      }
+
+    // ID3v1 tag is empty. Remove the old one.
+
+    if(d->ID3v1Location >= 0) {
+      truncate(d->ID3v1Location);
+      d->ID3v1Location = -1;
     }
   }
 
   // Update APE tag
 
-  if(APETag()) {
-    if(d->hasAPE)
-      insert(APETag()->render(), d->APELocation, d->APESize);
-    else {
-      if(d->hasID3v1)  {
-        insert(APETag()->render(), d->ID3v1Location, 0);
-        d->APESize = APETag()->footer()->completeTagSize();
-        d->hasAPE = true;
+  if(APETag() && !APETag()->isEmpty()) {
+
+    // APE tag is not empty. Update the old one or create a new one.
+
+    if(d->APELocation < 0) {
+      if(d->ID3v1Location >= 0)
         d->APELocation = d->ID3v1Location;
-        d->ID3v1Location += d->APESize;
-      }
-      else {
-        seek(0, End);
-        d->APELocation = tell();
-        writeBlock(APETag()->render());
-        d->APESize = APETag()->footer()->completeTagSize();
-        d->hasAPE = true;
-      }
+      else
+        d->APELocation = length();
     }
+
+    const ByteVector data = APETag()->render();
+    insert(data, d->APELocation, d->APESize);
+
+    if(d->ID3v1Location >= 0)
+      d->ID3v1Location += (data.size() - d->APESize);
+
+    d->APESize = data.size();
   }
   else {
-    if(d->hasAPE) {
+
+    // APE tag is empty. Remove the old one.
+
+    if(d->APELocation >= 0) {
       removeBlock(d->APELocation, d->APESize);
-      d->hasAPE = false;
-      if(d->hasID3v1) {
-        if(d->ID3v1Location > d->APELocation) {
-          d->ID3v1Location -= d->APESize;
-        }
-      }
+
+      if(d->ID3v1Location >= 0)
+        d->ID3v1Location -= d->APESize;
+
+      d->APELocation = -1;
+      d->APESize = 0;
     }
   }
 
-   return true;
+  return true;
 }
 
 ID3v1::Tag *APE::File::ID3v1Tag(bool create)
@@ -227,27 +220,24 @@ APE::Tag *APE::File::APETag(bool create)
 
 void APE::File::strip(int tags)
 {
-  if(tags & ID3v1) {
+  if(tags & ID3v1)
     d->tag.set(ApeID3v1Index, 0);
-    APETag(true);
-  }
 
-  if(tags & APE) {
+  if(tags & APE)
     d->tag.set(ApeAPEIndex, 0);
 
-    if(!ID3v1Tag())
-      APETag(true);
-  }
+  if(!ID3v1Tag())
+    APETag(true);
 }
 
 bool APE::File::hasAPETag() const
 {
-  return d->hasAPE;
+  return (d->APELocation >= 0);
 }
 
 bool APE::File::hasID3v1Tag() const
 {
-  return d->hasID3v1;
+  return (d->ID3v1Location >= 0);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -264,17 +254,14 @@ void APE::File::read(bool readProperties)
     seek(d->ID3v2Location);
     d->ID3v2Header = new ID3v2::Header(readBlock(ID3v2::Header::size()));
     d->ID3v2Size = d->ID3v2Header->completeTagSize();
-    d->hasID3v2 = true;
   }
 
   // Look for an ID3v1 tag
 
   d->ID3v1Location = Utils::findID3v1(this);
 
-  if(d->ID3v1Location >= 0) {
+  if(d->ID3v1Location >= 0)
     d->tag.set(ApeID3v1Index, new ID3v1::Tag(this, d->ID3v1Location));
-    d->hasID3v1 = true;
-  }
 
   // Look for an APE tag
 
@@ -284,10 +271,9 @@ void APE::File::read(bool readProperties)
     d->tag.set(ApeAPEIndex, new APE::Tag(this, d->APELocation));
     d->APESize = APETag()->footer()->completeTagSize();
     d->APELocation = d->APELocation + APE::Footer::size() - d->APESize;
-    d->hasAPE = true;
   }
 
-  if(!d->hasID3v1)
+  if(d->ID3v1Location < 0)
     APETag(true);
 
   // Look for APE audio properties
@@ -296,14 +282,14 @@ void APE::File::read(bool readProperties)
 
     long streamLength;
 
-    if(d->hasAPE)
+    if(d->APELocation >= 0)
       streamLength = d->APELocation;
-    else if(d->hasID3v1)
+    else if(d->ID3v1Location >= 0)
       streamLength = d->ID3v1Location;
     else
       streamLength = length();
 
-    if(d->hasID3v2) {
+    if(d->ID3v2Location >= 0) {
       seek(d->ID3v2Location + d->ID3v2Size);
       streamLength -= (d->ID3v2Location + d->ID3v2Size);
     }
diff --git a/taglib/ape/apefile.h b/taglib/ape/apefile.h
index e638d865..cfb19ff7 100644
--- a/taglib/ape/apefile.h
+++ b/taglib/ape/apefile.h
@@ -146,9 +146,6 @@ namespace TagLib {
        *
        * \note According to the official Monkey's Audio SDK, an APE file
        * can only have either ID3V1 or APE tags, so a parameter is used here.
-       *
-       * \warning In the current implementation, it's dangerous to call save()
-       * repeatedly.  At worst it will corrupt the file.
        */
       virtual bool save();
 
diff --git a/tests/test_ape.cpp b/tests/test_ape.cpp
index 64869c4c..12ca2db8 100644
--- a/tests/test_ape.cpp
+++ b/tests/test_ape.cpp
@@ -23,6 +23,7 @@ class TestAPE : public CppUnit::TestFixture
   CPPUNIT_TEST(testFuzzedFile1);
   CPPUNIT_TEST(testFuzzedFile2);
   CPPUNIT_TEST(testStripAndProperties);
+  CPPUNIT_TEST(testRepeatedSave);
   CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -134,6 +135,32 @@ public:
     }
   }
 
+  void testRepeatedSave()
+  {
+    ScopedFileCopy copy("mac-399", ".ape");
+
+    {
+      APE::File f(copy.fileName().c_str());
+      CPPUNIT_ASSERT(!f.hasAPETag());
+      CPPUNIT_ASSERT(!f.hasID3v1Tag());
+
+      f.APETag(true)->setTitle("01234 56789 ABCDE FGHIJ");
+      f.save();
+
+      f.APETag()->setTitle("0");
+      f.save();
+
+      f.ID3v1Tag(true)->setTitle("01234 56789 ABCDE FGHIJ");
+      f.APETag()->setTitle("01234 56789 ABCDE FGHIJ 01234 56789 ABCDE FGHIJ 01234 56789");
+      f.save();
+    }
+    {
+      APE::File f(copy.fileName().c_str());
+      CPPUNIT_ASSERT(f.hasAPETag());
+      CPPUNIT_ASSERT(f.hasID3v1Tag());
+    }
+  }
+
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(TestAPE);

From 083eadec609da79de3bcba6d18e3e479a1fa3025 Mon Sep 17 00:00:00 2001
From: Tsuda Kageyu <tsuda.kageyu@gmail.com>
Date: Fri, 27 Nov 2015 00:52:44 +0900
Subject: [PATCH 03/19] Fix saving MPC files.

This fixes the issue reported at #624.
---
 taglib/mpc/mpcfile.cpp | 181 ++++++++++++++++++++---------------------
 taglib/mpc/mpcfile.h   |   3 -
 tests/test_mpc.cpp     |  27 ++++++
 3 files changed, 114 insertions(+), 97 deletions(-)

diff --git a/taglib/mpc/mpcfile.cpp b/taglib/mpc/mpcfile.cpp
index 015bc0e0..83a9be82 100644
--- a/taglib/mpc/mpcfile.cpp
+++ b/taglib/mpc/mpcfile.cpp
@@ -53,10 +53,7 @@ public:
     ID3v2Header(0),
     ID3v2Location(-1),
     ID3v2Size(0),
-    properties(0),
-    hasAPE(false),
-    hasID3v1(false),
-    hasID3v2(false) {}
+    properties(0) {}
 
   ~FilePrivate()
   {
@@ -76,13 +73,6 @@ public:
   TagUnion tag;
 
   Properties *properties;
-
-  // These indicate whether the file *on disk* has these tags, not if
-  // this data structure does.  This is used in computing offsets.
-
-  bool hasAPE;
-  bool hasID3v1;
-  bool hasID3v2;
 };
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -147,69 +137,80 @@ bool MPC::File::save()
 
   // Possibly strip ID3v2 tag
 
-  if(d->hasID3v2 && !d->ID3v2Header) {
+  if(!d->ID3v2Header && d->ID3v2Location >= 0) {
     removeBlock(d->ID3v2Location, d->ID3v2Size);
-    d->hasID3v2 = false;
-    if(d->hasID3v1)
-      d->ID3v1Location -= d->ID3v2Size;
-    if(d->hasAPE)
+
+    if(d->APELocation >= 0)
       d->APELocation -= d->ID3v2Size;
+
+    if(d->ID3v1Location >= 0)
+      d->ID3v1Location -= d->ID3v2Size;
+
+    d->ID3v2Location = -1;
+    d->ID3v2Size = 0;
   }
 
   // Update ID3v1 tag
 
-  if(ID3v1Tag()) {
-    if(d->hasID3v1) {
+  if(ID3v1Tag() && !ID3v1Tag()->isEmpty()) {
+
+    // ID3v1 tag is not empty. Update the old one or create a new one.
+
+    if(d->ID3v1Location >= 0) {
       seek(d->ID3v1Location);
-      writeBlock(ID3v1Tag()->render());
     }
     else {
       seek(0, End);
       d->ID3v1Location = tell();
-      writeBlock(ID3v1Tag()->render());
-      d->hasID3v1 = true;
     }
-  } else
-    if(d->hasID3v1) {
-      removeBlock(d->ID3v1Location, 128);
-      d->hasID3v1 = false;
-      if(d->hasAPE) {
-        if(d->APELocation > d->ID3v1Location)
-          d->APELocation -= 128;
-      }
+
+    writeBlock(ID3v1Tag()->render());
+  }
+  else {
+
+    // ID3v1 tag is empty. Remove the old one.
+
+    if(d->ID3v1Location >= 0) {
+      truncate(d->ID3v1Location);
+      d->ID3v1Location = -1;
     }
+  }
 
   // Update APE tag
 
-  if(APETag()) {
-    if(d->hasAPE)
-      insert(APETag()->render(), d->APELocation, d->APESize);
-    else {
-      if(d->hasID3v1)  {
-        insert(APETag()->render(), d->ID3v1Location, 0);
-        d->APESize = APETag()->footer()->completeTagSize();
-        d->hasAPE = true;
+  if(APETag() && !APETag()->isEmpty()) {
+
+    // APE tag is not empty. Update the old one or create a new one.
+
+    if(d->APELocation < 0) {
+      if(d->ID3v1Location >= 0)
         d->APELocation = d->ID3v1Location;
-        d->ID3v1Location += d->APESize;
-      }
-      else {
-        seek(0, End);
-        d->APELocation = tell();
-        writeBlock(APETag()->render());
-        d->APESize = APETag()->footer()->completeTagSize();
-        d->hasAPE = true;
-      }
+      else
+        d->APELocation = length();
+    }
+
+    const ByteVector data = APETag()->render();
+    insert(data, d->APELocation, d->APESize);
+
+    if(d->ID3v1Location >= 0)
+      d->ID3v1Location += (data.size() - d->APESize);
+
+    d->APESize = data.size();
+  }
+  else {
+
+    // APE tag is empty. Remove the old one.
+
+    if(d->APELocation >= 0) {
+      removeBlock(d->APELocation, d->APESize);
+
+      if(d->ID3v1Location >= 0)
+        d->ID3v1Location -= d->APESize;
+
+      d->APELocation = -1;
+      d->APESize = 0;
     }
   }
-  else
-    if(d->hasAPE) {
-      removeBlock(d->APELocation, d->APESize);
-      d->hasAPE = false;
-      if(d->hasID3v1) {
-        if(d->ID3v1Location > d->APELocation)
-          d->ID3v1Location -= d->APESize;
-      }
-    }
 
   return true;
 }
@@ -226,22 +227,19 @@ APE::Tag *MPC::File::APETag(bool create)
 
 void MPC::File::strip(int tags)
 {
-  if(tags & ID3v1) {
+  if(tags & ID3v1)
     d->tag.set(MPCID3v1Index, 0);
+
+  if(tags & APE)
+    d->tag.set(MPCAPEIndex, 0);
+
+  if(!ID3v1Tag())
     APETag(true);
-  }
 
   if(tags & ID3v2) {
     delete d->ID3v2Header;
     d->ID3v2Header = 0;
   }
-
-  if(tags & APE) {
-    d->tag.set(MPCAPEIndex, 0);
-
-    if(!ID3v1Tag())
-      APETag(true);
-  }
 }
 
 void MPC::File::remove(int tags)
@@ -251,12 +249,12 @@ void MPC::File::remove(int tags)
 
 bool MPC::File::hasID3v1Tag() const
 {
-  return d->hasID3v1;
+  return (d->ID3v1Location >= 0);
 }
 
 bool MPC::File::hasAPETag() const
 {
-  return d->hasAPE;
+  return (d->APELocation >= 0);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -265,30 +263,6 @@ bool MPC::File::hasAPETag() const
 
 void MPC::File::read(bool readProperties)
 {
-  // Look for an ID3v1 tag
-
-  d->ID3v1Location = Utils::findID3v1(this);
-
-  if(d->ID3v1Location >= 0) {
-    d->tag.set(MPCID3v1Index, new ID3v1::Tag(this, d->ID3v1Location));
-    d->hasID3v1 = true;
-  }
-
-  // Look for an APE tag
-
-  d->APELocation = Utils::findAPE(this, d->ID3v1Location);
-
-  if(d->APELocation >= 0) {
-    d->tag.set(MPCAPEIndex, new APE::Tag(this, d->APELocation));
-
-    d->APESize = APETag()->footer()->completeTagSize();
-    d->APELocation = d->APELocation + APE::Footer::size() - d->APESize;
-    d->hasAPE = true;
-  }
-
-  if(!d->hasID3v1)
-    APETag(true);
-
   // Look for an ID3v2 tag
 
   d->ID3v2Location = Utils::findID3v2(this);
@@ -297,23 +271,42 @@ void MPC::File::read(bool readProperties)
     seek(d->ID3v2Location);
     d->ID3v2Header = new ID3v2::Header(readBlock(ID3v2::Header::size()));
     d->ID3v2Size = d->ID3v2Header->completeTagSize();
-    d->hasID3v2 = true;
   }
 
+  // Look for an ID3v1 tag
+
+  d->ID3v1Location = Utils::findID3v1(this);
+
+  if(d->ID3v1Location >= 0)
+    d->tag.set(MPCID3v1Index, new ID3v1::Tag(this, d->ID3v1Location));
+
+  // Look for an APE tag
+
+  d->APELocation = Utils::findAPE(this, d->ID3v1Location);
+
+  if(d->APELocation >= 0) {
+    d->tag.set(MPCAPEIndex, new APE::Tag(this, d->APELocation));
+    d->APESize = APETag()->footer()->completeTagSize();
+    d->APELocation = d->APELocation + APE::Footer::size() - d->APESize;
+  }
+
+  if(d->ID3v1Location < 0)
+    APETag(true);
+
   // Look for MPC metadata
 
   if(readProperties) {
 
     long streamLength;
 
-    if(d->hasAPE)
+    if(d->APELocation >= 0)
       streamLength = d->APELocation;
-    else if(d->hasID3v1)
+    else if(d->ID3v1Location >= 0)
       streamLength = d->ID3v1Location;
     else
       streamLength = length();
 
-    if(d->hasID3v2) {
+    if(d->ID3v2Location >= 0) {
       seek(d->ID3v2Location + d->ID3v2Size);
       streamLength -= (d->ID3v2Location + d->ID3v2Size);
     }
diff --git a/taglib/mpc/mpcfile.h b/taglib/mpc/mpcfile.h
index 98e7480f..541724dc 100644
--- a/taglib/mpc/mpcfile.h
+++ b/taglib/mpc/mpcfile.h
@@ -141,9 +141,6 @@ namespace TagLib {
        * Saves the file.
        *
        * This returns true if the save was successful.
-       *
-       * \warning In the current implementation, it's dangerous to call save()
-       * repeatedly.  At worst it will corrupt the file.
        */
       virtual bool save();
 
diff --git a/tests/test_mpc.cpp b/tests/test_mpc.cpp
index ec1e9f1c..94613c1e 100644
--- a/tests/test_mpc.cpp
+++ b/tests/test_mpc.cpp
@@ -24,6 +24,7 @@ class TestMPC : public CppUnit::TestFixture
   CPPUNIT_TEST(testFuzzedFile3);
   CPPUNIT_TEST(testFuzzedFile4);
   CPPUNIT_TEST(testStripAndProperties);
+  CPPUNIT_TEST(testRepeatedSave);
   CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -132,6 +133,32 @@ public:
     }
   }
 
+  void testRepeatedSave()
+  {
+    ScopedFileCopy copy("click", ".mpc");
+
+    {
+      MPC::File f(copy.fileName().c_str());
+      CPPUNIT_ASSERT(!f.hasAPETag());
+      CPPUNIT_ASSERT(!f.hasID3v1Tag());
+
+      f.APETag(true)->setTitle("01234 56789 ABCDE FGHIJ");
+      f.save();
+
+      f.APETag()->setTitle("0");
+      f.save();
+
+      f.ID3v1Tag(true)->setTitle("01234 56789 ABCDE FGHIJ");
+      f.APETag()->setTitle("01234 56789 ABCDE FGHIJ 01234 56789 ABCDE FGHIJ 01234 56789");
+      f.save();
+    }
+    {
+      MPC::File f(copy.fileName().c_str());
+      CPPUNIT_ASSERT(f.hasAPETag());
+      CPPUNIT_ASSERT(f.hasID3v1Tag());
+    }
+  }
+
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(TestMPC);

From 905bf89e667f600e161654de8e6dbd5b789d7691 Mon Sep 17 00:00:00 2001
From: Tsuda Kageyu <tsuda.kageyu@gmail.com>
Date: Fri, 27 Nov 2015 09:26:24 +0900
Subject: [PATCH 04/19] Fix saving WavPack files.

This fixes the issue reported at #624.
---
 taglib/wavpack/wavpackfile.cpp | 113 +++++++++++++++------------------
 taglib/wavpack/wavpackfile.h   |   3 -
 tests/test_wavpack.cpp         |  27 ++++++++
 3 files changed, 78 insertions(+), 65 deletions(-)

diff --git a/taglib/wavpack/wavpackfile.cpp b/taglib/wavpack/wavpackfile.cpp
index f7c97f7e..2ec55716 100644
--- a/taglib/wavpack/wavpackfile.cpp
+++ b/taglib/wavpack/wavpackfile.cpp
@@ -54,9 +54,7 @@ public:
     APELocation(-1),
     APESize(0),
     ID3v1Location(-1),
-    properties(0),
-    hasAPE(false),
-    hasID3v1(false) {}
+    properties(0) {}
 
   ~FilePrivate()
   {
@@ -71,12 +69,6 @@ public:
   TagUnion tag;
 
   Properties *properties;
-
-  // These indicate whether the file *on disk* has these tags, not if
-  // this data structure does.  This is used in computing offsets.
-
-  bool hasAPE;
-  bool hasID3v1;
 };
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -141,64 +133,67 @@ bool WavPack::File::save()
 
   // Update ID3v1 tag
 
-  if(ID3v1Tag()) {
-    if(d->hasID3v1) {
+  if(ID3v1Tag() && !ID3v1Tag()->isEmpty()) {
+
+    // ID3v1 tag is not empty. Update the old one or create a new one.
+
+    if(d->ID3v1Location >= 0) {
       seek(d->ID3v1Location);
-      writeBlock(ID3v1Tag()->render());
     }
     else {
       seek(0, End);
       d->ID3v1Location = tell();
-      writeBlock(ID3v1Tag()->render());
-      d->hasID3v1 = true;
     }
+
+    writeBlock(ID3v1Tag()->render());
   }
   else {
-    if(d->hasID3v1) {
-      removeBlock(d->ID3v1Location, 128);
-      d->hasID3v1 = false;
-      if(d->hasAPE) {
-        if(d->APELocation > d->ID3v1Location)
-          d->APELocation -= 128;
-      }
+
+    // ID3v1 tag is empty. Remove the old one.
+
+    if(d->ID3v1Location >= 0) {
+      truncate(d->ID3v1Location);
+      d->ID3v1Location = -1;
     }
   }
 
   // Update APE tag
 
-  if(APETag()) {
-    if(d->hasAPE)
-      insert(APETag()->render(), d->APELocation, d->APESize);
-    else {
-      if(d->hasID3v1)  {
-        insert(APETag()->render(), d->ID3v1Location, 0);
-        d->APESize = APETag()->footer()->completeTagSize();
-        d->hasAPE = true;
+  if(APETag() && !APETag()->isEmpty()) {
+
+    // APE tag is not empty. Update the old one or create a new one.
+
+    if(d->APELocation < 0) {
+      if(d->ID3v1Location >= 0)
         d->APELocation = d->ID3v1Location;
-        d->ID3v1Location += d->APESize;
-      }
-      else {
-        seek(0, End);
-        d->APELocation = tell();
-        writeBlock(APETag()->render());
-        d->APESize = APETag()->footer()->completeTagSize();
-        d->hasAPE = true;
-      }
+      else
+        d->APELocation = length();
     }
+
+    const ByteVector data = APETag()->render();
+    insert(data, d->APELocation, d->APESize);
+
+    if(d->ID3v1Location >= 0)
+      d->ID3v1Location += (data.size() - d->APESize);
+
+    d->APESize = data.size();
   }
   else {
-    if(d->hasAPE) {
+
+    // APE tag is empty. Remove the old one.
+
+    if(d->APELocation >= 0) {
       removeBlock(d->APELocation, d->APESize);
-      d->hasAPE = false;
-      if(d->hasID3v1) {
-        if(d->ID3v1Location > d->APELocation) {
-          d->ID3v1Location -= d->APESize;
-        }
-      }
+
+      if(d->ID3v1Location >= 0)
+        d->ID3v1Location -= d->APESize;
+
+      d->APELocation = -1;
+      d->APESize = 0;
     }
   }
 
-   return true;
+  return true;
 }
 
 ID3v1::Tag *WavPack::File::ID3v1Tag(bool create)
@@ -213,27 +208,24 @@ APE::Tag *WavPack::File::APETag(bool create)
 
 void WavPack::File::strip(int tags)
 {
-  if(tags & ID3v1) {
+  if(tags & ID3v1)
     d->tag.set(WavID3v1Index, 0);
-    APETag(true);
-  }
 
-  if(tags & APE) {
+  if(tags & APE)
     d->tag.set(WavAPEIndex, 0);
 
-    if(!ID3v1Tag())
-      APETag(true);
-  }
+  if(!ID3v1Tag())
+    APETag(true);
 }
 
 bool WavPack::File::hasID3v1Tag() const
 {
-  return d->hasID3v1;
+  return (d->ID3v1Location >= 0);
 }
 
 bool WavPack::File::hasAPETag() const
 {
-  return d->hasAPE;
+  return (d->APELocation >= 0);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -246,10 +238,8 @@ void WavPack::File::read(bool readProperties)
 
   d->ID3v1Location = Utils::findID3v1(this);
 
-  if(d->ID3v1Location >= 0) {
+  if(d->ID3v1Location >= 0)
     d->tag.set(WavID3v1Index, new ID3v1::Tag(this, d->ID3v1Location));
-    d->hasID3v1 = true;
-  }
 
   // Look for an APE tag
 
@@ -259,10 +249,9 @@ void WavPack::File::read(bool readProperties)
     d->tag.set(WavAPEIndex, new APE::Tag(this, d->APELocation));
     d->APESize = APETag()->footer()->completeTagSize();
     d->APELocation = d->APELocation + APE::Footer::size() - d->APESize;
-    d->hasAPE = true;
   }
 
-  if(!d->hasID3v1)
+  if(d->ID3v1Location >= 0)
     APETag(true);
 
   // Look for WavPack audio properties
@@ -271,9 +260,9 @@ void WavPack::File::read(bool readProperties)
 
     long streamLength;
 
-    if(d->hasAPE)
+    if(d->APELocation >= 0)
       streamLength = d->APELocation;
-    else if(d->hasID3v1)
+    else if(d->ID3v1Location >= 0)
       streamLength = d->ID3v1Location;
     else
       streamLength = length();
diff --git a/taglib/wavpack/wavpackfile.h b/taglib/wavpack/wavpackfile.h
index abcabd94..7e0bd27a 100644
--- a/taglib/wavpack/wavpackfile.h
+++ b/taglib/wavpack/wavpackfile.h
@@ -135,9 +135,6 @@ namespace TagLib {
        * Saves the file.
        *
        * This returns true if the save was successful.
-       *
-       * \warning In the current implementation, it's dangerous to call save()
-       * repeatedly.  At worst it will corrupt the file.
        */
       virtual bool save();
 
diff --git a/tests/test_wavpack.cpp b/tests/test_wavpack.cpp
index eddc601d..39bc1edd 100644
--- a/tests/test_wavpack.cpp
+++ b/tests/test_wavpack.cpp
@@ -19,6 +19,7 @@ class TestWavPack : public CppUnit::TestFixture
   CPPUNIT_TEST(testTaggedProperties);
   CPPUNIT_TEST(testFuzzedFile);
   CPPUNIT_TEST(testStripAndProperties);
+  CPPUNIT_TEST(testRepeatedSave);
   CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -97,6 +98,32 @@ public:
     }
   }
 
+  void testRepeatedSave()
+  {
+    ScopedFileCopy copy("click", ".wv");
+
+    {
+      WavPack::File f(copy.fileName().c_str());
+      CPPUNIT_ASSERT(!f.hasAPETag());
+      CPPUNIT_ASSERT(!f.hasID3v1Tag());
+
+      f.APETag(true)->setTitle("01234 56789 ABCDE FGHIJ");
+      f.save();
+
+      f.APETag()->setTitle("0");
+      f.save();
+
+      f.ID3v1Tag(true)->setTitle("01234 56789 ABCDE FGHIJ");
+      f.APETag()->setTitle("01234 56789 ABCDE FGHIJ 01234 56789 ABCDE FGHIJ 01234 56789");
+      f.save();
+    }
+    {
+      WavPack::File f(copy.fileName().c_str());
+      CPPUNIT_ASSERT(f.hasAPETag());
+      CPPUNIT_ASSERT(f.hasID3v1Tag());
+    }
+  }
+
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(TestWavPack);

From 31d1f11969f40a6d8580201d60a1824cbbffdf45 Mon Sep 17 00:00:00 2001
From: Tsuda Kageyu <tsuda.kageyu@gmail.com>
Date: Mon, 30 Nov 2015 15:00:32 +0900
Subject: [PATCH 05/19] Use a const pointer to initialize a const pointer.

---
 taglib/mpeg/mpegfile.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp
index 6fbbf19e..23eeeb5e 100644
--- a/taglib/mpeg/mpegfile.cpp
+++ b/taglib/mpeg/mpegfile.cpp
@@ -65,7 +65,7 @@ namespace
 class MPEG::File::FilePrivate
 {
 public:
-  FilePrivate(ID3v2::FrameFactory *frameFactory = ID3v2::FrameFactory::instance()) :
+  FilePrivate(const ID3v2::FrameFactory *frameFactory = ID3v2::FrameFactory::instance()) :
     ID3v2FrameFactory(frameFactory),
     ID3v2Location(-1),
     ID3v2OriginalSize(0),

From 9950fca3c28db6d56a5514db962cdeabff691adb Mon Sep 17 00:00:00 2001
From: Tsuda Kageyu <tsuda.kageyu@gmail.com>
Date: Wed, 16 Dec 2015 10:00:08 +0900
Subject: [PATCH 06/19] Remove duplicate Vorbis comment blocks when saving a
 FLAC file.

---
 taglib/flac/flacfile.cpp | 97 +++++++++++++++++-----------------------
 tests/test_flac.cpp      |  2 +
 2 files changed, 42 insertions(+), 57 deletions(-)

diff --git a/taglib/flac/flacfile.cpp b/taglib/flac/flacfile.cpp
index bdb77d52..63f460da 100644
--- a/taglib/flac/flacfile.cpp
+++ b/taglib/flac/flacfile.cpp
@@ -175,21 +175,16 @@ bool FLAC::File::save()
 
   // Replace metadata blocks
 
-  bool foundVorbisCommentBlock = false;
-
   for(BlockIterator it = d->blocks.begin(); it != d->blocks.end(); ++it) {
     if((*it)->code() == MetadataBlock::VorbisComment) {
       // Set the new Vorbis Comment block
       delete *it;
-      *it = new UnknownMetadataBlock(MetadataBlock::VorbisComment, d->xiphCommentData);
-      foundVorbisCommentBlock = true;
+      d->blocks.erase(it);
+      break;
     }
   }
 
-  if(!foundVorbisCommentBlock) {
-    d->blocks.append(new UnknownMetadataBlock(MetadataBlock::VorbisComment, d->xiphCommentData));
-    foundVorbisCommentBlock = true;
-  }
+  d->blocks.append(new UnknownMetadataBlock(MetadataBlock::VorbisComment, d->xiphCommentData));
 
   // Render data for the metadata blocks
 
@@ -469,51 +464,43 @@ void FLAC::File::scan()
   nextBlockOffset += 4;
   d->flacStart = nextBlockOffset;
 
-  seek(nextBlockOffset);
+  while(true) {
 
-  ByteVector header = readBlock(4);
+    seek(nextBlockOffset);
+    const ByteVector header = readBlock(4);
 
-  // Header format (from spec):
-  // <1> Last-metadata-block flag
-  // <7> BLOCK_TYPE
-  //    0 : STREAMINFO
-  //    1 : PADDING
-  //    ..
-  //    4 : VORBIS_COMMENT
-  //    ..
-  // <24> Length of metadata to follow
+    // Header format (from spec):
+    // <1> Last-metadata-block flag
+    // <7> BLOCK_TYPE
+    //    0 : STREAMINFO
+    //    1 : PADDING
+    //    ..
+    //    4 : VORBIS_COMMENT
+    //    ..
+    //    6 : PICTURE
+    //    ..
+    // <24> Length of metadata to follow
 
-  char blockType = header[0] & 0x7f;
-  bool isLastBlock = (header[0] & 0x80) != 0;
-  unsigned int length = header.toUInt(1U, 3U);
+    const char blockType = header[0] & ~LastBlockFlag;
+    const bool isLastBlock = (header[0] & LastBlockFlag) != 0;
+    const unsigned int blockLength = header.toUInt(1U, 3U);
 
-  // First block should be the stream_info metadata
+    // First block should be the stream_info metadata
 
-  if(blockType != MetadataBlock::StreamInfo) {
-    debug("FLAC::File::scan() -- invalid FLAC stream");
-    setValid(false);
-    return;
-  }
+    if(d->blocks.isEmpty() && blockType != MetadataBlock::StreamInfo) {
+      debug("FLAC::File::scan() -- First block should be the stream_info metadata");
+      setValid(false);
+      return;
+    }
 
-  d->blocks.append(new UnknownMetadataBlock(blockType, readBlock(length)));
-  nextBlockOffset += length + 4;
-
-  // Search through the remaining metadata
-  while(!isLastBlock) {
-
-    header = readBlock(4);
-    blockType = header[0] & 0x7f;
-    isLastBlock = (header[0] & 0x80) != 0;
-    length = header.toUInt(1U, 3U);
-
-    if(length == 0 && blockType != MetadataBlock::Padding) {
+    if(blockLength == 0 && blockType != MetadataBlock::Padding) {
       debug("FLAC::File::scan() -- Zero-sized metadata block found");
       setValid(false);
       return;
     }
 
-    const ByteVector data = readBlock(length);
-    if(data.size() != length) {
+    const ByteVector data = readBlock(blockLength);
+    if(data.size() != blockLength) {
       debug("FLAC::File::scan() -- Failed to read a metadata block");
       setValid(false);
       return;
@@ -525,9 +512,10 @@ void FLAC::File::scan()
     if(blockType == MetadataBlock::VorbisComment) {
       if(d->xiphCommentData.isEmpty()) {
         d->xiphCommentData = data;
+        block = new UnknownMetadataBlock(MetadataBlock::VorbisComment, data);
       }
       else {
-        debug("FLAC::File::scan() -- multiple Vorbis Comment blocks found, using the first one");
+        debug("FLAC::File::scan() -- multiple Vorbis Comment blocks found, discarding");
       }
     }
     else if(blockType == MetadataBlock::Picture) {
@@ -540,25 +528,20 @@ void FLAC::File::scan()
         delete picture;
       }
     }
-
-    if(!block) {
-      block = new UnknownMetadataBlock(blockType, data);
-    }
-    if(block->code() != MetadataBlock::Padding) {
-      d->blocks.append(block);
+    else if(blockType == MetadataBlock::Padding) {
+      // Skip all padding blocks.
     }
     else {
-      delete block;
+      block = new UnknownMetadataBlock(blockType, data);
     }
 
-    nextBlockOffset += length + 4;
+    if(block)
+      d->blocks.append(block);
 
-    if(nextBlockOffset >= File::length()) {
-      debug("FLAC::File::scan() -- FLAC stream corrupted");
-      setValid(false);
-      return;
-    }
-    seek(nextBlockOffset);
+    nextBlockOffset += blockLength + 4;
+
+    if(isLastBlock)
+      break;
   }
 
   // End of metadata, now comes the datastream
diff --git a/tests/test_flac.cpp b/tests/test_flac.cpp
index 97abe590..5bc98e7a 100644
--- a/tests/test_flac.cpp
+++ b/tests/test_flac.cpp
@@ -60,6 +60,8 @@ public:
     {
       FLAC::File f(newname.c_str());
       CPPUNIT_ASSERT_EQUAL(String("The Artist"), f.tag()->artist());
+      CPPUNIT_ASSERT_EQUAL(69L, f.find("Artist"));
+      CPPUNIT_ASSERT_EQUAL(-1L, f.find("Artist", 70));
     }
   }
 

From 581e58585ab234654984c7477b6abe01f9592cd4 Mon Sep 17 00:00:00 2001
From: Tsuda Kageyu <tsuda.kageyu@gmail.com>
Date: Wed, 16 Dec 2015 13:39:22 +0900
Subject: [PATCH 07/19] Amend a comment that refers to a deprecated function.

---
 taglib/mpeg/id3v2/id3v2framefactory.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/taglib/mpeg/id3v2/id3v2framefactory.h b/taglib/mpeg/id3v2/id3v2framefactory.h
index fe81c71f..c3875dc5 100644
--- a/taglib/mpeg/id3v2/id3v2framefactory.h
+++ b/taglib/mpeg/id3v2/id3v2framefactory.h
@@ -47,9 +47,9 @@ namespace TagLib {
      * Reimplementing this factory is the key to adding support for frame types
      * not directly supported by TagLib to your application.  To do so you would
      * subclass this factory reimplement createFrame().  Then by setting your
-     * factory to be the default factory in ID3v2::Tag constructor or with
-     * MPEG::File::setID3v2FrameFactory() you can implement behavior that will
-     * allow for new ID3v2::Frame subclasses (also provided by you) to be used.
+     * factory to be the default factory in ID3v2::Tag constructor you can
+     * implement behavior that will allow for new ID3v2::Frame subclasses (also
+     * provided by you) to be used.
      *
      * This implements both <i>abstract factory</i> and <i>singleton</i> patterns
      * of which more information is available on the web and in software design

From f0b0b736e3be8191284aa28a154072c8da2556d3 Mon Sep 17 00:00:00 2001
From: Tsuda Kageyu <tsuda.kageyu@gmail.com>
Date: Thu, 17 Dec 2015 11:26:45 +0900
Subject: [PATCH 08/19] Update NEWS.

---
 NEWS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/NEWS b/NEWS
index 6d7c485c..0f20d024 100644
--- a/NEWS
+++ b/NEWS
@@ -15,6 +15,7 @@
  * Fixed possible file corruptions when saving ASF files.
  * Fixed possible file corruptions when saving FLAC files.
  * Fixed possible file corruptions when saving MP4 files.
+ * Fixed possible file corruptions when saving MPEG files.
  * Fixed updating the comment field of Vorbis comments.
  * Fixed reading date and time in ID3v2.3 tags.
  * Replaced ENABLE_STATIC build option with BUILD_SHARED_LIBS.

From 862952bdccd588ca4b262f94d0971bf82bf1c0bb Mon Sep 17 00:00:00 2001
From: Tsuda Kageyu <tsuda.kageyu@gmail.com>
Date: Thu, 17 Dec 2015 11:43:11 +0900
Subject: [PATCH 09/19] Remove unnecessary private data members from
 TrueAudio::File.

---
 taglib/trueaudio/trueaudiofile.cpp | 103 ++++++++++++++---------------
 tests/test_trueaudio.cpp           |  27 ++++++++
 2 files changed, 78 insertions(+), 52 deletions(-)

diff --git a/taglib/trueaudio/trueaudiofile.cpp b/taglib/trueaudio/trueaudiofile.cpp
index 7910c485..27dc606d 100644
--- a/taglib/trueaudio/trueaudiofile.cpp
+++ b/taglib/trueaudio/trueaudiofile.cpp
@@ -55,9 +55,7 @@ public:
     ID3v2Location(-1),
     ID3v2OriginalSize(0),
     ID3v1Location(-1),
-    properties(0),
-    hasID3v1(false),
-    hasID3v2(false) {}
+    properties(0) {}
 
   ~FilePrivate()
   {
@@ -73,12 +71,6 @@ public:
   TagUnion tag;
 
   Properties *properties;
-
-  // These indicate whether the file *on disk* has these tags, not if
-  // this data structure does.  This is used in computing offsets.
-
-  bool hasID3v1;
-  bool hasID3v2;
 };
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -167,40 +159,59 @@ bool TrueAudio::File::save()
   // Update ID3v2 tag
 
   if(ID3v2Tag() && !ID3v2Tag()->isEmpty()) {
-    if(!d->hasID3v2) {
+
+    // ID3v2 tag is not empty. Update the old one or create a new one.
+
+    if(d->ID3v2Location < 0)
       d->ID3v2Location = 0;
+
+    const ByteVector data = ID3v2Tag()->render();
+    insert(data, d->ID3v2Location, d->ID3v2OriginalSize);
+
+    if(d->ID3v1Location >= 0)
+      d->ID3v1Location += (data.size() - d->ID3v2OriginalSize);
+
+    d->ID3v2OriginalSize = data.size();
+  }
+  else {
+
+    // ID3v2 tag is empty. Remove the old one.
+
+    if(d->ID3v2Location >= 0) {
+      removeBlock(d->ID3v2Location, d->ID3v2OriginalSize);
+
+      if(d->ID3v1Location >= 0)
+        d->ID3v1Location -= d->ID3v2OriginalSize;
+
+      d->ID3v2Location = -1;
       d->ID3v2OriginalSize = 0;
     }
-    ByteVector data = ID3v2Tag()->render();
-    insert(data, d->ID3v2Location, d->ID3v2OriginalSize);
-    d->ID3v1Location -= d->ID3v2OriginalSize - data.size();
-    d->ID3v2OriginalSize = data.size();
-    d->hasID3v2 = true;
-  }
-  else if(d->hasID3v2) {
-    removeBlock(d->ID3v2Location, d->ID3v2OriginalSize);
-    d->ID3v1Location -= d->ID3v2OriginalSize;
-    d->ID3v2Location = -1;
-    d->ID3v2OriginalSize = 0;
-    d->hasID3v2 = false;
   }
 
   // Update ID3v1 tag
 
   if(ID3v1Tag() && !ID3v1Tag()->isEmpty()) {
-    if(!d->hasID3v1) {
+
+    // ID3v1 tag is not empty. Update the old one or create a new one.
+
+    if(d->ID3v1Location >= 0) {
+      seek(d->ID3v1Location);
+    }
+    else {
       seek(0, End);
       d->ID3v1Location = tell();
     }
-    else
-      seek(d->ID3v1Location);
+
     writeBlock(ID3v1Tag()->render());
-    d->hasID3v1 = true;
   }
-  else if(d->hasID3v1) {
-    removeBlock(d->ID3v1Location, 128);
-    d->ID3v1Location = -1;
-    d->hasID3v1 = false;
+  else {
+
+    // ID3v1 tag is empty. Remove the old one.
+
+    if(d->ID3v1Location >= 0) {
+      truncate(d->ID3v1Location);
+      d->ID3v1Location = -1;
+    }
   }
 
   return true;
@@ -218,27 +229,24 @@ ID3v2::Tag *TrueAudio::File::ID3v2Tag(bool create)
 
 void TrueAudio::File::strip(int tags)
 {
-  if(tags & ID3v1) {
+  if(tags & ID3v1)
     d->tag.set(TrueAudioID3v1Index, 0);
-    ID3v2Tag(true);
-  }
 
-  if(tags & ID3v2) {
+  if(tags & ID3v2)
     d->tag.set(TrueAudioID3v2Index, 0);
 
-    if(!ID3v1Tag())
-      ID3v2Tag(true);
-  }
+  if(!ID3v1Tag())
+    ID3v2Tag(true);
 }
 
 bool TrueAudio::File::hasID3v1Tag() const
 {
-  return d->hasID3v1;
+  return (d->ID3v1Location >= 0);
 }
 
 bool TrueAudio::File::hasID3v2Tag() const
 {
-  return d->hasID3v2;
+  return (d->ID3v2Location >= 0);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -252,27 +260,18 @@ void TrueAudio::File::read(bool readProperties)
   d->ID3v2Location = Utils::findID3v2(this);
 
   if(d->ID3v2Location >= 0) {
-
     d->tag.set(TrueAudioID3v2Index, new ID3v2::Tag(this, d->ID3v2Location, d->ID3v2FrameFactory));
-
     d->ID3v2OriginalSize = ID3v2Tag()->header()->completeTagSize();
-
-    if(ID3v2Tag()->header()->tagSize() <= 0)
-      d->tag.set(TrueAudioID3v2Index, 0);
-    else
-      d->hasID3v2 = true;
   }
 
   // Look for an ID3v1 tag
 
   d->ID3v1Location = Utils::findID3v1(this);
 
-  if(d->ID3v1Location >= 0) {
+  if(d->ID3v1Location >= 0)
     d->tag.set(TrueAudioID3v1Index, new ID3v1::Tag(this, d->ID3v1Location));
-    d->hasID3v1 = true;
-  }
 
-  if(!d->hasID3v1)
+  if(d->ID3v1Location < 0)
     ID3v2Tag(true);
 
   // Look for TrueAudio metadata
@@ -281,12 +280,12 @@ void TrueAudio::File::read(bool readProperties)
 
     long streamLength;
 
-    if(d->hasID3v1)
+    if(d->ID3v1Location >= 0)
       streamLength = d->ID3v1Location;
     else
       streamLength = length();
 
-    if(d->hasID3v2) {
+    if(d->ID3v2Location >= 0) {
       seek(d->ID3v2Location + d->ID3v2OriginalSize);
       streamLength -= (d->ID3v2Location + d->ID3v2OriginalSize);
     }
diff --git a/tests/test_trueaudio.cpp b/tests/test_trueaudio.cpp
index 8468e0ec..c960f5f7 100644
--- a/tests/test_trueaudio.cpp
+++ b/tests/test_trueaudio.cpp
@@ -16,6 +16,7 @@ class TestTrueAudio : public CppUnit::TestFixture
   CPPUNIT_TEST(testReadPropertiesWithoutID3v2);
   CPPUNIT_TEST(testReadPropertiesWithTags);
   CPPUNIT_TEST(testStripAndProperties);
+  CPPUNIT_TEST(testRepeatedSave);
   CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -70,6 +71,32 @@ public:
     }
   }
 
+  void testRepeatedSave()
+  {
+    ScopedFileCopy copy("empty", ".tta");
+
+    {
+      TrueAudio::File f(copy.fileName().c_str());
+      CPPUNIT_ASSERT(!f.hasID3v2Tag());
+      CPPUNIT_ASSERT(!f.hasID3v1Tag());
+
+      f.ID3v2Tag(true)->setTitle("01234 56789 ABCDE FGHIJ");
+      f.save();
+
+      f.ID3v2Tag()->setTitle("0");
+      f.save();
+
+      f.ID3v1Tag(true)->setTitle("01234 56789 ABCDE FGHIJ");
+      f.ID3v2Tag()->setTitle("01234 56789 ABCDE FGHIJ 01234 56789 ABCDE FGHIJ 01234 56789");
+      f.save();
+    }
+    {
+      TrueAudio::File f(copy.fileName().c_str());
+      CPPUNIT_ASSERT(f.hasID3v2Tag());
+      CPPUNIT_ASSERT(f.hasID3v1Tag());
+    }
+  }
+
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(TestTrueAudio);

From 50711f47586e189ec4d937a0808d240022b7295c Mon Sep 17 00:00:00 2001
From: Tsuda Kageyu <tsuda.kageyu@gmail.com>
Date: Thu, 17 Dec 2015 16:59:28 +0900
Subject: [PATCH 10/19] Update NEWS.

---
 NEWS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/NEWS b/NEWS
index 0f20d024..85c027e4 100644
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,7 @@
  * Fixed possible file corruptions when saving FLAC files.
  * Fixed possible file corruptions when saving MP4 files.
  * Fixed possible file corruptions when saving MPEG files.
+ * Fixed possible file corruptions when saving APE files.
  * Fixed updating the comment field of Vorbis comments.
  * Fixed reading date and time in ID3v2.3 tags.
  * Replaced ENABLE_STATIC build option with BUILD_SHARED_LIBS.

From 7c5d1e02decef52af772e7679f17c372a61c2468 Mon Sep 17 00:00:00 2001
From: Tsuda Kageyu <tsuda.kageyu@gmail.com>
Date: Fri, 18 Dec 2015 09:03:31 +0900
Subject: [PATCH 11/19] Update NEWS.

---
 NEWS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/NEWS b/NEWS
index 85c027e4..96a50ea9 100644
--- a/NEWS
+++ b/NEWS
@@ -17,6 +17,7 @@
  * Fixed possible file corruptions when saving MP4 files.
  * Fixed possible file corruptions when saving MPEG files.
  * Fixed possible file corruptions when saving APE files.
+ * Fixed possible file corruptions when saving Musepack files.
  * Fixed updating the comment field of Vorbis comments.
  * Fixed reading date and time in ID3v2.3 tags.
  * Replaced ENABLE_STATIC build option with BUILD_SHARED_LIBS.

From 8b6bcf411a3b0df955b2664c234bc62e42573553 Mon Sep 17 00:00:00 2001
From: Tsuda Kageyu <tsuda.kageyu@gmail.com>
Date: Fri, 18 Dec 2015 13:54:47 +0900
Subject: [PATCH 12/19] Update NEWS.

---
 NEWS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/NEWS b/NEWS
index 96a50ea9..5d3f2c48 100644
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,7 @@
  * Fixed possible file corruptions when saving MPEG files.
  * Fixed possible file corruptions when saving APE files.
  * Fixed possible file corruptions when saving Musepack files.
+ * Fixed possible file corruptions when saving WavPack files.
  * Fixed updating the comment field of Vorbis comments.
  * Fixed reading date and time in ID3v2.3 tags.
  * Replaced ENABLE_STATIC build option with BUILD_SHARED_LIBS.

From 4ec264b6a137db1fdf452ebc183eeb24f6e74d7a Mon Sep 17 00:00:00 2001
From: Tsuda Kageyu <tsuda.kageyu@gmail.com>
Date: Mon, 21 Dec 2015 10:05:05 +0900
Subject: [PATCH 13/19] Update NEWS.

---
 NEWS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/NEWS b/NEWS
index 5d3f2c48..89839d83 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,7 @@
  * Better handling of duplicate ID3v2 tags in all kinds of files.
  * Better handling of duplicate tag chunks in WAV files.
  * Better handling of duplicate tag chunks in AIFF files.
+ * Better handling of duplicate Vorbis comment blocks in FLAC files.
  * Fixed crash when calling File::properties() after strip().
  * Fixed crash when parsing certain MPEG files.
  * Fixed possible file corruptions when saving ASF files.

From cf5d431d772c653c6f05aac8830f7acfb859249f Mon Sep 17 00:00:00 2001
From: Tsuda Kageyu <tsuda.kageyu@gmail.com>
Date: Mon, 21 Dec 2015 10:49:46 +0900
Subject: [PATCH 14/19] Remove an unused private data member.

---
 taglib/mpeg/mpegfile.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp
index 5865763f..e1910c2d 100644
--- a/taglib/mpeg/mpegfile.cpp
+++ b/taglib/mpeg/mpegfile.cpp
@@ -85,7 +85,6 @@ public:
   unsigned int ID3v2OriginalSize;
 
   long APELocation;
-  long APEFooterLocation;
   unsigned int APEOriginalSize;
 
   long ID3v1Location;

From d0238ba82fcbc40895075bf32d946ec6fb7018c0 Mon Sep 17 00:00:00 2001
From: Tsuda Kageyu <tsuda.kageyu@gmail.com>
Date: Mon, 21 Dec 2015 11:44:25 +0900
Subject: [PATCH 15/19] Avoid the risk of subtracting between signed and
 unsigned types.

---
 taglib/ape/apefile.cpp             |  6 +++---
 taglib/flac/flacfile.cpp           | 12 ++++++------
 taglib/mpc/mpcfile.cpp             |  6 +++---
 taglib/mpeg/mpegfile.cpp           |  6 +++---
 taglib/trueaudio/trueaudiofile.cpp |  4 ++--
 taglib/wavpack/wavpackfile.cpp     |  4 ++--
 6 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/taglib/ape/apefile.cpp b/taglib/ape/apefile.cpp
index c0e32c33..9f298aaf 100644
--- a/taglib/ape/apefile.cpp
+++ b/taglib/ape/apefile.cpp
@@ -70,13 +70,13 @@ public:
   }
 
   long APELocation;
-  unsigned int APESize;
+  long APESize;
 
   long ID3v1Location;
 
   ID3v2::Header *ID3v2Header;
   long ID3v2Location;
-  unsigned int ID3v2Size;
+  long ID3v2Size;
 
   TagUnion tag;
 
@@ -186,7 +186,7 @@ bool APE::File::save()
     insert(data, d->APELocation, d->APESize);
 
     if(d->ID3v1Location >= 0)
-      d->ID3v1Location += (data.size() - d->APESize);
+      d->ID3v1Location += (static_cast<long>(data.size()) - d->APESize);
 
     d->APESize = data.size();
   }
diff --git a/taglib/flac/flacfile.cpp b/taglib/flac/flacfile.cpp
index 2e58e848..fb7418d1 100644
--- a/taglib/flac/flacfile.cpp
+++ b/taglib/flac/flacfile.cpp
@@ -80,7 +80,7 @@ public:
 
   const ID3v2::FrameFactory *ID3v2FrameFactory;
   long ID3v2Location;
-  unsigned int ID3v2OriginalSize;
+  long ID3v2OriginalSize;
 
   long ID3v1Location;
 
@@ -226,10 +226,10 @@ bool FLAC::File::save()
 
   insert(data, d->flacStart, originalLength);
 
-  d->streamStart += (data.size() - originalLength);
+  d->streamStart += (static_cast<long>(data.size()) - originalLength);
 
   if(d->ID3v1Location >= 0)
-    d->ID3v1Location += (data.size() - originalLength);
+    d->ID3v1Location += (static_cast<long>(data.size()) - originalLength);
 
   // Update ID3 tags
 
@@ -243,11 +243,11 @@ bool FLAC::File::save()
     data = ID3v2Tag()->render();
     insert(data, d->ID3v2Location, d->ID3v2OriginalSize);
 
-    d->flacStart   += (data.size() - d->ID3v2OriginalSize);
-    d->streamStart += (data.size() - d->ID3v2OriginalSize);
+    d->flacStart   += (static_cast<long>(data.size()) - d->ID3v2OriginalSize);
+    d->streamStart += (static_cast<long>(data.size()) - d->ID3v2OriginalSize);
 
     if(d->ID3v1Location >= 0)
-      d->ID3v1Location += (data.size() - d->ID3v2OriginalSize);
+      d->ID3v1Location += (static_cast<long>(data.size()) - d->ID3v2OriginalSize);
 
     d->ID3v2OriginalSize = data.size();
   }
diff --git a/taglib/mpc/mpcfile.cpp b/taglib/mpc/mpcfile.cpp
index b586bbf6..daf24c8f 100644
--- a/taglib/mpc/mpcfile.cpp
+++ b/taglib/mpc/mpcfile.cpp
@@ -62,13 +62,13 @@ public:
   }
 
   long APELocation;
-  unsigned int APESize;
+  long APESize;
 
   long ID3v1Location;
 
   ID3v2::Header *ID3v2Header;
   long ID3v2Location;
-  unsigned int ID3v2Size;
+  long ID3v2Size;
 
   TagUnion tag;
 
@@ -193,7 +193,7 @@ bool MPC::File::save()
     insert(data, d->APELocation, d->APESize);
 
     if(d->ID3v1Location >= 0)
-      d->ID3v1Location += (data.size() - d->APESize);
+      d->ID3v1Location += (static_cast<long>(data.size()) - d->APESize);
 
     d->APESize = data.size();
   }
diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp
index e1910c2d..4fc3533f 100644
--- a/taglib/mpeg/mpegfile.cpp
+++ b/taglib/mpeg/mpegfile.cpp
@@ -82,10 +82,10 @@ public:
   const ID3v2::FrameFactory *ID3v2FrameFactory;
 
   long ID3v2Location;
-  unsigned int ID3v2OriginalSize;
+  long ID3v2OriginalSize;
 
   long APELocation;
-  unsigned int APEOriginalSize;
+  long APEOriginalSize;
 
   long ID3v1Location;
 
@@ -274,7 +274,7 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version, bool duplica
       insert(data, d->APELocation, d->APEOriginalSize);
 
       if(d->ID3v1Location >= 0)
-        d->ID3v1Location += (data.size() - d->APEOriginalSize);
+        d->ID3v1Location += (static_cast<long>(data.size()) - d->APEOriginalSize);
 
       d->APEOriginalSize = data.size();
     }
diff --git a/taglib/trueaudio/trueaudiofile.cpp b/taglib/trueaudio/trueaudiofile.cpp
index 27dc606d..fc123ba3 100644
--- a/taglib/trueaudio/trueaudiofile.cpp
+++ b/taglib/trueaudio/trueaudiofile.cpp
@@ -64,7 +64,7 @@ public:
 
   const ID3v2::FrameFactory *ID3v2FrameFactory;
   long ID3v2Location;
-  unsigned int ID3v2OriginalSize;
+  long ID3v2OriginalSize;
 
   long ID3v1Location;
 
@@ -169,7 +169,7 @@ bool TrueAudio::File::save()
     insert(data, d->ID3v2Location, d->ID3v2OriginalSize);
 
     if(d->ID3v1Location >= 0)
-      d->ID3v1Location += (data.size() - d->ID3v2OriginalSize);
+      d->ID3v1Location += (static_cast<long>(data.size()) - d->ID3v2OriginalSize);
 
     d->ID3v2OriginalSize = data.size();
   }
diff --git a/taglib/wavpack/wavpackfile.cpp b/taglib/wavpack/wavpackfile.cpp
index 909c5590..ef92f4bd 100644
--- a/taglib/wavpack/wavpackfile.cpp
+++ b/taglib/wavpack/wavpackfile.cpp
@@ -62,7 +62,7 @@ public:
   }
 
   long APELocation;
-  unsigned int APESize;
+  long APESize;
 
   long ID3v1Location;
 
@@ -174,7 +174,7 @@ bool WavPack::File::save()
     insert(data, d->APELocation, d->APESize);
 
     if(d->ID3v1Location >= 0)
-      d->ID3v1Location += (data.size() - d->APESize);
+      d->ID3v1Location += (static_cast<long>(data.size()) - d->APESize);
 
     d->APESize = data.size();
   }

From dd2ed47703f23905d11944dea246ac975bf84ba9 Mon Sep 17 00:00:00 2001
From: Tsuda Kageyu <tsuda.kageyu@gmail.com>
Date: Mon, 21 Dec 2015 13:06:43 +0900
Subject: [PATCH 16/19] Avoid an implicit const cast.

---
 taglib/mpeg/id3v2/id3v2frame.cpp | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/taglib/mpeg/id3v2/id3v2frame.cpp b/taglib/mpeg/id3v2/id3v2frame.cpp
index 8960ffe2..52180447 100644
--- a/taglib/mpeg/id3v2/id3v2frame.cpp
+++ b/taglib/mpeg/id3v2/id3v2frame.cpp
@@ -265,17 +265,19 @@ ByteVector Frame::fieldData(const ByteVector &frameData) const
     if(inflateInit(&stream) != Z_OK)
       return ByteVector();
 
-    stream.avail_in = (uLongf) frameData.size() - frameDataOffset;
-    stream.next_in = (Bytef *) frameData.data() + frameDataOffset;
+    ByteVector inData = frameData;
+
+    stream.avail_in = static_cast<uInt>(inData.size() - frameDataOffset);
+    stream.next_in = reinterpret_cast<Bytef *>(inData.data() + frameDataOffset);
 
     static const unsigned int chunkSize = 1024;
 
-    ByteVector data;
+    ByteVector outData;
     ByteVector chunk(chunkSize);
 
     do {
-      stream.avail_out = (uLongf) chunk.size();
-      stream.next_out = (Bytef *) chunk.data();
+      stream.avail_out = static_cast<uInt>(chunk.size());
+      stream.next_out = reinterpret_cast<Bytef *>(chunk.data());
 
       int result = inflate(&stream, Z_NO_FLUSH);
 
@@ -290,15 +292,15 @@ ByteVector Frame::fieldData(const ByteVector &frameData) const
         return ByteVector();
       }
 
-      data.append(stream.avail_out == 0 ? chunk : chunk.mid(0, chunk.size() - stream.avail_out));
+      outData.append(stream.avail_out == 0 ? chunk : chunk.mid(0, chunk.size() - stream.avail_out));
     } while(stream.avail_out == 0);
 
     inflateEnd(&stream);
 
-    if(frameDataLength != data.size())
+    if(frameDataLength != outData.size())
       debug("frameDataLength does not match the data length returned by zlib");
 
-    return data;
+    return outData;
   }
   else
 #endif

From 5cd139a578b12a0b05fde599eb7d40bbb24fc65d Mon Sep 17 00:00:00 2001
From: Tsuda Kageyu <tsuda.kageyu@gmail.com>
Date: Mon, 21 Dec 2015 13:34:48 +0900
Subject: [PATCH 17/19] Small fix in style.

---
 ConfigureChecks.cmake | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ConfigureChecks.cmake b/ConfigureChecks.cmake
index d0ad1eaa..3d2632c0 100644
--- a/ConfigureChecks.cmake
+++ b/ConfigureChecks.cmake
@@ -209,7 +209,7 @@ check_cxx_source_compiles("
   int main() {
     _strdup(0);
     return 0;
-}
+  }
 " HAVE_ISO_STRDUP)
 
 # Check for libz using the cmake supplied FindZLIB.cmake

From 130654316bd039f8b7d9132246b2dadcebcca8d6 Mon Sep 17 00:00:00 2001
From: Tsuda Kageyu <tsuda.kageyu@gmail.com>
Date: Mon, 21 Dec 2015 13:54:02 +0900
Subject: [PATCH 18/19] Backport some comments from tablib2 branch.

---
 ConfigureChecks.cmake | 4 +++-
 config.h.cmake        | 4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/ConfigureChecks.cmake b/ConfigureChecks.cmake
index 3d2632c0..a9f9f17a 100644
--- a/ConfigureChecks.cmake
+++ b/ConfigureChecks.cmake
@@ -212,7 +212,7 @@ check_cxx_source_compiles("
   }
 " HAVE_ISO_STRDUP)
 
-# Check for libz using the cmake supplied FindZLIB.cmake
+# Determine whether zlib is installed.
 
 if(NOT ZLIB_SOURCE)
   find_package(ZLIB)
@@ -223,6 +223,8 @@ if(NOT ZLIB_SOURCE)
   endif()
 endif()
 
+# Determine whether CppUnit is installed.
+
 if(BUILD_TESTS AND NOT BUILD_SHARED_LIBS)
   find_package(CppUnit)
   if(NOT CppUnit_FOUND)
diff --git a/config.h.cmake b/config.h.cmake
index 55affab6..7ac72410 100644
--- a/config.h.cmake
+++ b/config.h.cmake
@@ -20,10 +20,10 @@
 #cmakedefine   HAVE_VSNPRINTF 1
 #cmakedefine   HAVE_VSPRINTF_S 1
 
-/* Defined if your compiler supports ISO _strdup. */
+/* Defined if your compiler supports ISO _strdup */
 #cmakedefine   HAVE_ISO_STRDUP 1
 
-/* Defined if you have libz */
+/* Defined if zlib is installed */
 #cmakedefine   HAVE_ZLIB 1
 
 /* Indicates whether debug messages are shown even in release mode */

From 5a1f44d166ff74255dbce0665efe43337256e6e9 Mon Sep 17 00:00:00 2001
From: Tsuda Kageyu <tsuda.kageyu@gmail.com>
Date: Mon, 21 Dec 2015 14:29:59 +0900
Subject: [PATCH 19/19] Avoid the risk of subtracting between signed and
 unsigned types.

---
 taglib/mpeg/mpegfile.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp
index 4fc3533f..67f0b273 100644
--- a/taglib/mpeg/mpegfile.cpp
+++ b/taglib/mpeg/mpegfile.cpp
@@ -218,10 +218,10 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version, bool duplica
       insert(data, d->ID3v2Location, d->ID3v2OriginalSize);
 
       if(d->APELocation >= 0)
-        d->APELocation += (data.size() - d->ID3v2OriginalSize);
+        d->APELocation += (static_cast<long>(data.size()) - d->ID3v2OriginalSize);
 
       if(d->ID3v1Location >= 0)
-        d->ID3v1Location += (data.size() - d->ID3v2OriginalSize);
+        d->ID3v1Location += (static_cast<long>(data.size()) - d->ID3v2OriginalSize);
 
       d->ID3v2OriginalSize = data.size();
     }