From 131918333b8972823c9f042b44eea57e25cf3f8a Mon Sep 17 00:00:00 2001 From: Urs Fleisch Date: Sat, 2 Dec 2023 11:44:00 +0100 Subject: [PATCH] Fix issues reported by Clang Static Analyzer Some deadcode.DeadStores. On Debian 12: CXXFLAGS=-I/usr/include/x86_64-linux-gnu/c++/12 \ cmake -DCMAKE_C_COMPILER=/usr/share/clang/scan-build-14/libexec/ccc-analyzer \ -DCMAKE_CXX_COMPILER=/usr/share/clang/scan-build-14/libexec/c++-analyzer \ -DBUILD_SHARED_LIBS=OFF \ -DCMAKE_EXPORT_COMPILE_COMMANDS=1 \ -DENABLE_CCACHE=ON -DBUILD_TESTING=ON -DBUILD_EXAMPLES=ON \ -DCMAKE_BUILD_TYPE=Debug ../taglib scan-build make --- bindings/c/tag_c.cpp | 158 ++++++++++++----------- taglib/mod/modfile.cpp | 31 +++-- taglib/mpc/mpcproperties.cpp | 1 - taglib/ogg/opus/opusproperties.cpp | 6 +- taglib/ogg/speex/speexproperties.cpp | 2 +- taglib/ogg/vorbis/vorbisproperties.cpp | 1 - taglib/trueaudio/trueaudioproperties.cpp | 1 - tests/test_id3v2.cpp | 5 +- tests/test_it.cpp | 2 + tests/test_mod.cpp | 2 + tests/test_s3m.cpp | 2 + tests/test_xm.cpp | 3 + 12 files changed, 117 insertions(+), 97 deletions(-) diff --git a/bindings/c/tag_c.cpp b/bindings/c/tag_c.cpp index c51766a3..b3fe5f3d 100644 --- a/bindings/c/tag_c.cpp +++ b/bindings/c/tag_c.cpp @@ -584,85 +584,91 @@ TagLib_Complex_Property_Attribute*** taglib_complex_property_get( TagLib_Complex_Property_Attribute ***propPtr = props; for(const auto &variantMap : variantMaps) { - TagLib_Complex_Property_Attribute **attrs = static_cast( - malloc(sizeof(TagLib_Complex_Property_Attribute *) * (variantMap.size() + 1))); - TagLib_Complex_Property_Attribute *attr = static_cast( - malloc(sizeof(TagLib_Complex_Property_Attribute) * variantMap.size())); - TagLib_Complex_Property_Attribute **attrPtr = attrs; - for (const auto &[k, v] : variantMap) { - attr->key = stringToCharArray(k); - attr->value.size = 0; - switch(v.type()) { - case Variant::Void: - attr->value.type = TagLib_Variant_Void; - attr->value.value.stringValue = NULL; - break; - case Variant::Bool: - attr->value.type = TagLib_Variant_Bool; - attr->value.value.boolValue = v.value(); - break; - case Variant::Int: - attr->value.type = TagLib_Variant_Int; - attr->value.value.intValue = v.value(); - break; - case Variant::UInt: - attr->value.type = TagLib_Variant_UInt; - attr->value.value.uIntValue = v.value(); - break; - case Variant::LongLong: - attr->value.type = TagLib_Variant_LongLong; - attr->value.value.longLongValue = v.value(); - break; - case Variant::ULongLong: - attr->value.type = TagLib_Variant_ULongLong; - attr->value.value.uLongLongValue = v.value(); - break; - case Variant::Double: - attr->value.type = TagLib_Variant_Double; - attr->value.value.doubleValue = v.value(); - break; - case Variant::String: { - attr->value.type = TagLib_Variant_String; - auto str = v.value(); - attr->value.value.stringValue = stringToCharArray(str); - attr->value.size = str.size(); - break; - } - case Variant::StringList: { - attr->value.type = TagLib_Variant_StringList; - StringList strs = v.value(); - auto strPtr = static_cast(malloc(sizeof(char *) * (strs.size() + 1))); - attr->value.value.stringListValue = strPtr; - attr->value.size = strs.size(); - for(const auto &str : strs) { - *strPtr++ = stringToCharArray(str); + if(!variantMap.isEmpty()) { + TagLib_Complex_Property_Attribute **attrs = static_cast( + malloc(sizeof(TagLib_Complex_Property_Attribute *) * (variantMap.size() + 1))); + TagLib_Complex_Property_Attribute *attr = static_cast( + malloc(sizeof(TagLib_Complex_Property_Attribute) * variantMap.size())); + TagLib_Complex_Property_Attribute **attrPtr = attrs; + // The next assignment is redundant to silence the clang analyzer, + // it is done at the end of the loop, which must be entered because + // variantMap is not empty. + *attrPtr = attr; + for(const auto &[k, v] : variantMap) { + attr->key = stringToCharArray(k); + attr->value.size = 0; + switch(v.type()) { + case Variant::Void: + attr->value.type = TagLib_Variant_Void; + attr->value.value.stringValue = NULL; + break; + case Variant::Bool: + attr->value.type = TagLib_Variant_Bool; + attr->value.value.boolValue = v.value(); + break; + case Variant::Int: + attr->value.type = TagLib_Variant_Int; + attr->value.value.intValue = v.value(); + break; + case Variant::UInt: + attr->value.type = TagLib_Variant_UInt; + attr->value.value.uIntValue = v.value(); + break; + case Variant::LongLong: + attr->value.type = TagLib_Variant_LongLong; + attr->value.value.longLongValue = v.value(); + break; + case Variant::ULongLong: + attr->value.type = TagLib_Variant_ULongLong; + attr->value.value.uLongLongValue = v.value(); + break; + case Variant::Double: + attr->value.type = TagLib_Variant_Double; + attr->value.value.doubleValue = v.value(); + break; + case Variant::String: { + attr->value.type = TagLib_Variant_String; + auto str = v.value(); + attr->value.value.stringValue = stringToCharArray(str); + attr->value.size = str.size(); + break; } - *strPtr = NULL; - break; + case Variant::StringList: { + attr->value.type = TagLib_Variant_StringList; + StringList strs = v.value(); + auto strPtr = static_cast(malloc(sizeof(char *) * (strs.size() + 1))); + attr->value.value.stringListValue = strPtr; + attr->value.size = strs.size(); + for(const auto &str : strs) { + *strPtr++ = stringToCharArray(str); + } + *strPtr = NULL; + break; + } + case Variant::ByteVector: { + attr->value.type = TagLib_Variant_ByteVector; + const ByteVector data = v.value(); + auto bytePtr = static_cast(malloc(data.size())); + attr->value.value.byteVectorValue = bytePtr; + attr->value.size = data.size(); + ::memcpy(bytePtr, data.data(), data.size()); + break; + } + case Variant::ByteVectorList: + case Variant::VariantList: + case Variant::VariantMap: { + attr->value.type = TagLib_Variant_String; + std::stringstream ss; + ss << v; + attr->value.value.stringValue = stringToCharArray(ss.str()); + break; + } + } + *attrPtr++ = attr++; } - case Variant::ByteVector: { - attr->value.type = TagLib_Variant_ByteVector; - const ByteVector data = v.value(); - auto bytePtr = static_cast(malloc(data.size())); - attr->value.value.byteVectorValue = bytePtr; - attr->value.size = data.size(); - ::memcpy(bytePtr, data.data(), data.size()); - break; - } - case Variant::ByteVectorList: - case Variant::VariantList: - case Variant::VariantMap: { - attr->value.type = TagLib_Variant_String; - std::stringstream ss; - ss << v; - attr->value.value.stringValue = stringToCharArray(ss.str()); - break; - } - } - *attrPtr++ = attr++; + *attrPtr = NULL; + *propPtr++ = attrs; } - *attrPtr++ = NULL; - *propPtr++ = attrs; } *propPtr = NULL; return props; diff --git a/taglib/mod/modfile.cpp b/taglib/mod/modfile.cpp index d73077a0..56ad8e54 100644 --- a/taglib/mod/modfile.cpp +++ b/taglib/mod/modfile.cpp @@ -160,25 +160,30 @@ void Mod::File::read(bool) seek(0); READ_STRING(d->tag.setTitle, 20); + offset_t pos = 20; StringList comment; for(unsigned int i = 0; i < instruments; ++ i) { READ_STRING_AS(instrumentName, 22); - // value in words, * 2 (<< 1) for bytes: - READ_U16B_AS(sampleLength); + // skip unused data + pos += 22 + 2 + 1 + 1 + 2 + 2; + seek(pos); - READ_BYTE_AS(fineTuneByte); - int fineTune = fineTuneByte & 0xF; - // > 7 means negative value - if(fineTune > 7) fineTune -= 16; + // // value in words, * 2 (<< 1) for bytes: + // READ_U16B_AS(sampleLength); - READ_BYTE_AS(volume); - if(volume > 64) volume = 64; - // volume in decibels: 20 * log10(volume / 64) + // READ_BYTE_AS(fineTuneByte); + // int fineTune = fineTuneByte & 0xF; + // // > 7 means negative value + // if(fineTune > 7) fineTune -= 16; - // value in words, * 2 (<< 1) for bytes: - READ_U16B_AS(repeatStart); - // value in words, * 2 (<< 1) for bytes: - READ_U16B_AS(repatLength); + // READ_BYTE_AS(volume); + // if(volume > 64) volume = 64; + // // volume in decibels: 20 * log10(volume / 64) + + // // value in words, * 2 (<< 1) for bytes: + // READ_U16B_AS(repeatStart); + // // value in words, * 2 (<< 1) for bytes: + // READ_U16B_AS(repeatLength); comment.append(instrumentName); } diff --git a/taglib/mpc/mpcproperties.cpp b/taglib/mpc/mpcproperties.cpp index 9f865f6b..d29cccba 100644 --- a/taglib/mpc/mpcproperties.cpp +++ b/taglib/mpc/mpcproperties.cpp @@ -221,7 +221,6 @@ void MPC::Properties::readSV8(File *file, offset_t streamLength) } const unsigned short flags = data.toUShort(pos, true); - pos += 2; d->sampleRate = sftable[(flags >> 13) & 0x07]; d->channels = ((flags >> 4) & 0x0F) + 1; diff --git a/taglib/ogg/opus/opusproperties.cpp b/taglib/ogg/opus/opusproperties.cpp index 88b080a7..ce1ecff8 100644 --- a/taglib/ogg/opus/opusproperties.cpp +++ b/taglib/ogg/opus/opusproperties.cpp @@ -122,13 +122,13 @@ void Opus::Properties::read(File *file) // *Input Sample Rate* (32 bits, unsigned, little endian) d->inputSampleRate = data.toUInt(pos, false); - pos += 4; + // pos += 4; // *Output Gain* (16 bits, signed, little endian) - pos += 2; + // pos += 2; // *Channel Mapping Family* (8 bits, unsigned) - pos += 1; + // pos += 1; const Ogg::PageHeader *first = file->firstPageHeader(); const Ogg::PageHeader *last = file->lastPageHeader(); diff --git a/taglib/ogg/speex/speexproperties.cpp b/taglib/ogg/speex/speexproperties.cpp index 2521ecbf..1843fcf8 100644 --- a/taglib/ogg/speex/speexproperties.cpp +++ b/taglib/ogg/speex/speexproperties.cpp @@ -141,8 +141,8 @@ void Speex::Properties::read(File *file) // vbr; /**< 1 for a VBR encoding, 0 otherwise */ d->vbr = data.toUInt(pos, false) == 1; - pos += 4; + // pos += 4; // frames_per_packet; /**< Number of frames stored per Ogg packet */ // unsigned int framesPerPacket = data.mid(pos, 4).toUInt(false); diff --git a/taglib/ogg/vorbis/vorbisproperties.cpp b/taglib/ogg/vorbis/vorbisproperties.cpp index 7e34b101..cbafc236 100644 --- a/taglib/ogg/vorbis/vorbisproperties.cpp +++ b/taglib/ogg/vorbis/vorbisproperties.cpp @@ -145,7 +145,6 @@ void Vorbis::Properties::read(File *file) pos += 4; d->bitrateMinimum = data.toUInt(pos, false); - pos += 4; // Find the length of the file. See http://wiki.xiph.org/VorbisStreamLength/ // for my notes on the topic. diff --git a/taglib/trueaudio/trueaudioproperties.cpp b/taglib/trueaudio/trueaudioproperties.cpp index 93fc5846..8f6bb0e8 100644 --- a/taglib/trueaudio/trueaudioproperties.cpp +++ b/taglib/trueaudio/trueaudioproperties.cpp @@ -141,7 +141,6 @@ void TrueAudio::Properties::read(const ByteVector &data, offset_t streamLength) pos += 4; d->sampleFrames = data.toUInt(pos, false); - pos += 4; if(d->sampleFrames > 0 && d->sampleRate > 0) { const double length = d->sampleFrames * 1000.0 / d->sampleRate; diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp index 6f857f06..e0489838 100644 --- a/tests/test_id3v2.cpp +++ b/tests/test_id3v2.cpp @@ -24,8 +24,8 @@ ***************************************************************************/ #include -#include #include +#include #include "tdebug.h" #include "tpropertymap.h" @@ -307,6 +307,7 @@ public: dynamic_cast(factory->createFrame(data, &header)); CPPUNIT_ASSERT(frame); + assert(frame != nullptr); // to silence the clang analyzer CPPUNIT_ASSERT_EQUAL(String("image/jpeg"), frame->mimeType()); CPPUNIT_ASSERT_EQUAL(ID3v2::AttachedPictureFrame::FileIcon, frame->type()); CPPUNIT_ASSERT_EQUAL(String("d"), frame->description()); @@ -1126,6 +1127,7 @@ public: ID3v2::AttachedPictureFrame *frame = dynamic_cast(f.ID3v2Tag()->frameListMap()["APIC"].front()); CPPUNIT_ASSERT(frame); + assert(frame != nullptr); // to silence the clang analyzer CPPUNIT_ASSERT_EQUAL(String("image/bmp"), frame->mimeType()); CPPUNIT_ASSERT_EQUAL(ID3v2::AttachedPictureFrame::Other, frame->type()); CPPUNIT_ASSERT_EQUAL(String(""), frame->description()); @@ -1148,6 +1150,7 @@ public: ID3v2::UrlLinkFrame *frame = dynamic_cast(f.ID3v2Tag()->frameListMap()["W000"].front()); CPPUNIT_ASSERT(frame); + assert(frame != nullptr); // to silence the clang analyzer CPPUNIT_ASSERT_EQUAL(String("lukas.lalinsky@example.com____"), frame->url()); } diff --git a/tests/test_it.cpp b/tests/test_it.cpp index 75b6370a..3faa6830 100644 --- a/tests/test_it.cpp +++ b/tests/test_it.cpp @@ -23,6 +23,7 @@ * http://www.mozilla.org/MPL/ * ***************************************************************************/ +#include #include "tstringlist.h" #include "itfile.h" #include @@ -106,6 +107,7 @@ private: CPPUNIT_ASSERT(nullptr != p); CPPUNIT_ASSERT(nullptr != t); + assert(p != nullptr); // to silence the clang analyzer CPPUNIT_ASSERT_EQUAL( 0, p->lengthInSeconds()); CPPUNIT_ASSERT_EQUAL( 0, p->bitrate()); diff --git a/tests/test_mod.cpp b/tests/test_mod.cpp index e1423124..e16eb8d7 100644 --- a/tests/test_mod.cpp +++ b/tests/test_mod.cpp @@ -23,6 +23,7 @@ * http://www.mozilla.org/MPL/ * ***************************************************************************/ +#include #include "tpropertymap.h" #include "modfile.h" #include @@ -112,6 +113,7 @@ private: CPPUNIT_ASSERT(nullptr != p); CPPUNIT_ASSERT(nullptr != t); + assert(p != nullptr); // to silence the clang analyzer CPPUNIT_ASSERT_EQUAL(0, p->lengthInSeconds()); CPPUNIT_ASSERT_EQUAL(0, p->bitrate()); diff --git a/tests/test_s3m.cpp b/tests/test_s3m.cpp index 0a225520..c57056e5 100644 --- a/tests/test_s3m.cpp +++ b/tests/test_s3m.cpp @@ -23,6 +23,7 @@ * http://www.mozilla.org/MPL/ * ***************************************************************************/ +#include #include "s3mfile.h" #include #include "utils.h" @@ -97,6 +98,7 @@ private: CPPUNIT_ASSERT(nullptr != p); CPPUNIT_ASSERT(nullptr != t); + assert(p != nullptr); // to silence the clang analyzer CPPUNIT_ASSERT_EQUAL( 0, p->lengthInSeconds()); CPPUNIT_ASSERT_EQUAL( 0, p->bitrate()); diff --git a/tests/test_xm.cpp b/tests/test_xm.cpp index 76b88837..8c4233a9 100644 --- a/tests/test_xm.cpp +++ b/tests/test_xm.cpp @@ -23,6 +23,7 @@ * http://www.mozilla.org/MPL/ * ***************************************************************************/ +#include #include "xmfile.h" #include #include "utils.h" @@ -129,6 +130,7 @@ public: CPPUNIT_ASSERT(nullptr != p); CPPUNIT_ASSERT(nullptr != t); + assert(p != nullptr); // to silence the clang analyzer CPPUNIT_ASSERT_EQUAL(0, p->lengthInSeconds()); CPPUNIT_ASSERT_EQUAL(0, p->bitrate()); @@ -175,6 +177,7 @@ private: CPPUNIT_ASSERT(nullptr != p); CPPUNIT_ASSERT(nullptr != t); + assert(p != nullptr); // to silence the clang analyzer CPPUNIT_ASSERT_EQUAL(0, p->lengthInSeconds()); CPPUNIT_ASSERT_EQUAL(0, p->bitrate());