From 4d126c49e97da8754ccf82c2ac10476c5fb94592 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Sun, 14 Jul 2013 02:28:57 +0900 Subject: [PATCH 1/4] Fixed a crash of APE::Item::toString() when the data type is binary --- examples/framelist.cpp | 5 ++++- taglib/ape/apeitem.cpp | 36 +++++++++++++++++++++++------------- taglib/ape/apeitem.h | 10 ++++++---- 3 files changed, 33 insertions(+), 18 deletions(-) diff --git a/examples/framelist.cpp b/examples/framelist.cpp index dbe80feb..679aa393 100644 --- a/examples/framelist.cpp +++ b/examples/framelist.cpp @@ -95,7 +95,10 @@ int main(int argc, char *argv[]) for(APE::ItemListMap::ConstIterator it = ape->itemListMap().begin(); it != ape->itemListMap().end(); ++it) { - cout << (*it).first << " - \"" << (*it).second.toString() << "\"" << endl; + if((*it).second.type() != APE::Item::Binary) + cout << (*it).first << " - \"" << (*it).second.toString() << "\"" << endl; + else + cout << (*it).first << " - Binary data (" << (*it).second.binaryData().size() << " bytes)" << endl; } } else diff --git a/taglib/ape/apeitem.cpp b/taglib/ape/apeitem.cpp index b1670a65..3490173a 100644 --- a/taglib/ape/apeitem.cpp +++ b/taglib/ape/apeitem.cpp @@ -125,6 +125,7 @@ void APE::Item::setBinaryData(const ByteVector &value) { d->type = Binary; d->value = value; + d->text.clear(); } ByteVector APE::Item::value() const @@ -137,31 +138,35 @@ ByteVector APE::Item::value() const void APE::Item::setKey(const String &key) { - d->key = key; + d->key = key; } void APE::Item::setValue(const String &value) { - d->type = Text; - d->text = value; + d->type = Text; + d->text = value; + d->value.clear(); } void APE::Item::setValues(const StringList &value) { - d->type = Text; - d->text = value; + d->type = Text; + d->text = value; + d->value.clear(); } void APE::Item::appendValue(const String &value) { - d->type = Text; - d->text.append(value); + d->type = Text; + d->text.append(value); + d->value.clear(); } void APE::Item::appendValues(const StringList &values) { - d->type = Text; - d->text.append(values); + d->type = Text; + d->text.append(values); + d->value.clear(); } int APE::Item::size() const @@ -200,7 +205,10 @@ StringList APE::Item::values() const String APE::Item::toString() const { - return isEmpty() ? String::null : d->text.front(); + if(d->type == Text && !isEmpty()) + return d->text.front(); + else + return String::null; } bool APE::Item::isEmpty() const @@ -234,13 +242,15 @@ void APE::Item::parse(const ByteVector &data) d->key = String(data.mid(8), String::UTF8); - d->value = data.mid(8 + d->key.size() + 1, valueLength); + const ByteVector value = data.mid(8 + d->key.size() + 1, valueLength); setReadOnly(flags & 1); setType(ItemTypes((flags >> 1) & 3)); - if(Text == d->type) - d->text = StringList(ByteVectorList::split(d->value, '\0'), String::UTF8); + if(Text == d->type) + d->text = StringList(ByteVectorList::split(value, '\0'), String::UTF8); + else + d->value = value; } ByteVector APE::Item::render() const diff --git a/taglib/ape/apeitem.h b/taglib/ape/apeitem.h index f7fd05e3..0588d185 100644 --- a/taglib/ape/apeitem.h +++ b/taglib/ape/apeitem.h @@ -97,7 +97,7 @@ namespace TagLib { /*! * Returns the binary value. - * If the item type is not \a Binary, the returned contents are undefined + * If the item type is not \a Binary, always returns an empty ByteVector. */ ByteVector binaryData() const; @@ -152,8 +152,9 @@ namespace TagLib { int size() const; /*! - * Returns the value as a single string. In case of multiple strings, - * the first is returned. + * Returns the value as a single string. In case of multiple strings, + * the first is returned. If the data type is not \a Text, always returns + * an empty String. */ String toString() const; @@ -163,7 +164,8 @@ namespace TagLib { #endif /*! - * Returns the list of text values. + * Returns the list of text values. If the data type is not \a Text, always + * returns an empty StringList. */ StringList values() const; From 0f58646bfbcbe13fa7732952c7a51156a10dbdd5 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Sun, 14 Jul 2013 11:22:15 +0900 Subject: [PATCH 2/4] Added a test for APE::Item --- tests/test_apetag.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/test_apetag.cpp b/tests/test_apetag.cpp index c93b2a7e..845828f5 100644 --- a/tests/test_apetag.cpp +++ b/tests/test_apetag.cpp @@ -20,6 +20,7 @@ class TestAPETag : public CppUnit::TestFixture CPPUNIT_TEST(testPropertyInterface1); CPPUNIT_TEST(testPropertyInterface2); CPPUNIT_TEST(testInvalidKeys); + CPPUNIT_TEST(testTextBinary); CPPUNIT_TEST_SUITE_END(); public: @@ -97,6 +98,23 @@ public: CPPUNIT_ASSERT(unsuccessful.contains("A")); CPPUNIT_ASSERT(unsuccessful.contains("MP+")); } + + void testTextBinary() + { + APE::Item item = APE::Item("DUMMY", "Test Text"); + CPPUNIT_ASSERT_EQUAL(String("Test Text"), item.toString()); + CPPUNIT_ASSERT_EQUAL(ByteVector::null, item.binaryData()); + + ByteVector data("Test Data"); + item.setBinaryData(data); + CPPUNIT_ASSERT(item.values().isEmpty()); + CPPUNIT_ASSERT_EQUAL(String::null, item.toString()); + CPPUNIT_ASSERT_EQUAL(data, item.binaryData()); + + item.setValue("Test Text 2"); + CPPUNIT_ASSERT_EQUAL(String("Test Text 2"), item.toString()); + CPPUNIT_ASSERT_EQUAL(ByteVector::null, item.binaryData()); + } }; From 35ca010df69e7fddbbd45a69648932210fbbaca9 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Wed, 17 Jul 2013 15:02:02 +0900 Subject: [PATCH 3/4] Introduced the runtime byte order detection when config.h is missing --- taglib/toolkit/tbytevector.cpp | 24 +++---- taglib/toolkit/tstring.cpp | 13 +--- taglib/toolkit/tutils.h | 115 ++++++++++++++++++++++----------- 3 files changed, 89 insertions(+), 63 deletions(-) diff --git a/taglib/toolkit/tbytevector.cpp b/taglib/toolkit/tbytevector.cpp index 0ea7517c..32308ff8 100644 --- a/taglib/toolkit/tbytevector.cpp +++ b/taglib/toolkit/tbytevector.cpp @@ -203,6 +203,9 @@ T toNumber(const ByteVector &v, size_t offset, size_t length, bool mostSignifica template T toNumber(const ByteVector &v, size_t offset, bool mostSignificantByteFirst) { + static const bool isBigEndian = (Utils::SystemByteOrder == Utils::BigEndian); + const bool swap = (mostSignificantByteFirst != isBigEndian); + if(offset + sizeof(T) > v.size()) return toNumber(v, offset, v.size() - offset, mostSignificantByteFirst); @@ -210,13 +213,8 @@ T toNumber(const ByteVector &v, size_t offset, bool mostSignificantByteFirst) T tmp; ::memcpy(&tmp, v.data() + offset, sizeof(T)); -#if SYSTEM_BYTEORDER == 1 - const bool swap = mostSignificantByteFirst; -#else - const bool swap != mostSignificantByteFirst; -#endif if(swap) - return byteSwap(tmp); + return Utils::byteSwap(tmp); else return tmp; } @@ -224,17 +222,13 @@ T toNumber(const ByteVector &v, size_t offset, bool mostSignificantByteFirst) template ByteVector fromNumber(T value, bool mostSignificantByteFirst) { - const size_t size = sizeof(T); + static const bool isBigEndian = (Utils::SystemByteOrder == Utils::BigEndian); + const bool swap = (mostSignificantByteFirst != isBigEndian); -#if SYSTEM_BYTEORDER == 1 - const bool swap = mostSignificantByteFirst; -#else - const bool swap != mostSignificantByteFirst; -#endif - if(swap) - value = byteSwap(value); + if(swap) + value = Utils::byteSwap(value); - return ByteVector(reinterpret_cast(&value), size); + return ByteVector(reinterpret_cast(&value), sizeof(T)); } class DataPrivate : public RefCounter diff --git a/taglib/toolkit/tstring.cpp b/taglib/toolkit/tstring.cpp index c83118d1..56840f48 100644 --- a/taglib/toolkit/tstring.cpp +++ b/taglib/toolkit/tstring.cpp @@ -800,7 +800,7 @@ void String::copyFromUTF16(const wchar_t *s, size_t length, Type t) if(swap) { for(size_t i = 0; i < length; ++i) - d->data[i] = byteSwap(static_cast(s[i])); + d->data[i] = Utils::byteSwap(static_cast(s[i])); } } @@ -839,15 +839,8 @@ void String::copyFromUTF16(const char *s, size_t length, Type t) } } -#if SYSTEM_BYTEORDER == 1 - -const String::Type String::WCharByteOrder = String::UTF16LE; - -#else - -const String::Type String::WCharByteOrder = String::UTF16BE; - -#endif +const String::Type String::WCharByteOrder + = (Utils::SystemByteOrder == Utils::BigEndian) ? String::UTF16BE : String::UTF16LE; } diff --git a/taglib/toolkit/tutils.h b/taglib/toolkit/tutils.h index 73c35716..4a398d26 100644 --- a/taglib/toolkit/tutils.h +++ b/taglib/toolkit/tutils.h @@ -46,105 +46,144 @@ namespace TagLib { - - inline ushort byteSwap(ushort x) + namespace Utils { + inline ushort byteSwap(ushort x) + { #if defined(HAVE_GCC_BYTESWAP_16) - return __builtin_bswap16(x); + return __builtin_bswap16(x); #elif defined(HAVE_MSC_BYTESWAP) - return _byteswap_ushort(x); + return _byteswap_ushort(x); #elif defined(HAVE_GLIBC_BYTESWAP) - return __bswap_16(x); + return __bswap_16(x); #elif defined(HAVE_MAC_BYTESWAP) - return OSSwapInt16(x); + return OSSwapInt16(x); #elif defined(HAVE_OPENBSD_BYTESWAP) - return swap16(x); + return swap16(x); #else - return ((x >> 8) & 0xff) | ((x & 0xff) << 8); + return ((x >> 8) & 0xff) | ((x & 0xff) << 8); #endif - } + } - inline uint byteSwap(uint x) - { + inline uint byteSwap(uint x) + { #if defined(HAVE_GCC_BYTESWAP_32) - return __builtin_bswap32(x); + return __builtin_bswap32(x); #elif defined(HAVE_MSC_BYTESWAP) - return _byteswap_ulong(x); + return _byteswap_ulong(x); #elif defined(HAVE_GLIBC_BYTESWAP) - return __bswap_32(x); + return __bswap_32(x); #elif defined(HAVE_MAC_BYTESWAP) - return OSSwapInt32(x); + return OSSwapInt32(x); #elif defined(HAVE_OPENBSD_BYTESWAP) - return swap32(x); + return swap32(x); #else - return ((x & 0xff000000u) >> 24) - | ((x & 0x00ff0000u) >> 8) - | ((x & 0x0000ff00u) << 8) - | ((x & 0x000000ffu) << 24); + return ((x & 0xff000000u) >> 24) + | ((x & 0x00ff0000u) >> 8) + | ((x & 0x0000ff00u) << 8) + | ((x & 0x000000ffu) << 24); #endif - } + } - inline ulonglong byteSwap(ulonglong x) - { + inline ulonglong byteSwap(ulonglong x) + { #if defined(HAVE_GCC_BYTESWAP_64) - return __builtin_bswap64(x); + return __builtin_bswap64(x); #elif defined(HAVE_MSC_BYTESWAP) - return _byteswap_uint64(x); + return _byteswap_uint64(x); #elif defined(HAVE_GLIBC_BYTESWAP) - return __bswap_64(x); + return __bswap_64(x); #elif defined(HAVE_MAC_BYTESWAP) - return OSSwapInt64(x); + return OSSwapInt64(x); #elif defined(HAVE_OPENBSD_BYTESWAP) - return swap64(x); + return swap64(x); #else - return ((x & 0xff00000000000000ull) >> 56) - | ((x & 0x00ff000000000000ull) >> 40) - | ((x & 0x0000ff0000000000ull) >> 24) - | ((x & 0x000000ff00000000ull) >> 8) - | ((x & 0x00000000ff000000ull) << 8) - | ((x & 0x0000000000ff0000ull) << 24) - | ((x & 0x000000000000ff00ull) << 40) - | ((x & 0x00000000000000ffull) << 56); + return ((x & 0xff00000000000000ull) >> 56) + | ((x & 0x00ff000000000000ull) >> 40) + | ((x & 0x0000ff0000000000ull) >> 24) + | ((x & 0x000000ff00000000ull) >> 8) + | ((x & 0x00000000ff000000ull) << 8) + | ((x & 0x0000000000ff0000ull) << 24) + | ((x & 0x000000000000ff00ull) << 40) + | ((x & 0x00000000000000ffull) << 56); + +#endif + } + + enum ByteOrder + { + LittleEndian, + BigEndian + }; + +#ifdef SYSTEM_BYTEORDER + +# if SYSTEM_BYTEORDER == 1 + + const ByteOrder SystemByteOrder = LittleEndian; + +# else + + const ByteOrder SystemByteOrder = BigEndian; + +# endif + +#else + + inline ByteOrder systemByteOrder() + { + union { + int i; + char c; + } u; + + u.i = 1; + if(u.c == 1) + return LittleEndian; + else + return BigEndian; + } + + const ByteOrder SystemByteOrder = systemByteOrder(); #endif } - -}; +} #endif From 89fcab5669013bd46b0ef7b7f6efbb8a21cd1ceb Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Wed, 17 Jul 2013 23:35:41 +0900 Subject: [PATCH 4/4] Fixed an MSVC specific runtime error only in debug mode --- taglib/toolkit/tbytevector.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/taglib/toolkit/tbytevector.cpp b/taglib/toolkit/tbytevector.cpp index 0ea7517c..ddc3d4f4 100644 --- a/taglib/toolkit/tbytevector.cpp +++ b/taglib/toolkit/tbytevector.cpp @@ -156,9 +156,8 @@ int findVector( for(size_t i = 0; i < patternSize - 1; ++i) lastOccurrence[static_cast(*(patternBegin + i))] = patternSize - i - 1; - for(TIterator it = dataBegin + patternSize - 1 + offset; - it < dataEnd; - it += lastOccurrence[static_cast(*it)]) + TIterator it = dataBegin + patternSize - 1 + offset; + while(true) { TIterator itBuffer = it; TIterator itPattern = patternBegin + patternSize - 1; @@ -176,6 +175,12 @@ int findVector( --itBuffer; --itPattern; } + + const size_t step = lastOccurrence[static_cast(*it)]; + if(dataEnd - step <= it) + break; + + it += step; } return -1;