From 9c507b9d201c8cd75f8f9cd7289c2eef66ca679b Mon Sep 17 00:00:00 2001 From: tbeu Date: Fri, 3 Nov 2017 20:43:49 +0100 Subject: [PATCH 1/9] Fix function declaration V762 It is possible a virtual function was overridden incorrectly. See third argument of function 'printStructure' in derived class 'TiffImage' and base class 'Image'. tiffimage.hpp 93 --- include/exiv2/tiffimage.hpp | 2 +- src/tiffimage.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/exiv2/tiffimage.hpp b/include/exiv2/tiffimage.hpp index 9fecaa3f..c610865b 100644 --- a/include/exiv2/tiffimage.hpp +++ b/include/exiv2/tiffimage.hpp @@ -89,7 +89,7 @@ namespace Exiv2 { not valid (does not look like data of the specific image type). @warning This function is not thread safe and intended for exiv2 -p{S|R} as a file debugging aid */ - virtual void printStructure(std::ostream& out, PrintStructureOption option,int depth=-1); + virtual void printStructure(std::ostream& out, PrintStructureOption option,int depth=0); /*! @brief Not supported. TIFF format does not contain a comment. diff --git a/src/tiffimage.cpp b/src/tiffimage.cpp index 9af4bbe9..c256b904 100644 --- a/src/tiffimage.cpp +++ b/src/tiffimage.cpp @@ -184,7 +184,7 @@ namespace Exiv2 { // recursively print the structure to /dev/null to ensure all metadata is in memory // must be recursive to handle NEFs which stores the raw image in a subIFDs std::ofstream devnull; - printStructure(devnull,kpsRecursive,0); + printStructure(devnull,kpsRecursive); ByteOrder bo = TiffParser::decode(exifData_, iptcData_, xmpData_, From 3674ce2c1db6091cbdfd7161a7c1ae58182de1eb Mon Sep 17 00:00:00 2001 From: tbeu Date: Fri, 3 Nov 2017 20:47:55 +0100 Subject: [PATCH 2/9] Remove superfluous assignment V519 The 'md_st' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 155, 156. easyaccess.cpp 156 --- src/easyaccess.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/easyaccess.cpp b/src/easyaccess.cpp index 626b28a0..80822efb 100644 --- a/src/easyaccess.cpp +++ b/src/easyaccess.cpp @@ -147,8 +147,7 @@ namespace Exiv2 { // ISO value (see EXIF 2.3 Annex G) long iso_tmp_val = -1; while (iso_tmp_val == -1 && (iso_val == 65535 || md == ed.end())) { - ExifData::const_iterator md_st = ed.end(); - md_st = findMetadatum(ed, sensitivityType, 1); + ExifData::const_iterator md_st = findMetadatum(ed, sensitivityType, 1); // no SensitivityType? exit with existing data if (md_st == ed.end()) break; From e5a4f1cf35184fc292030ee03cb29ac19c0e99fc Mon Sep 17 00:00:00 2001 From: tbeu Date: Fri, 3 Nov 2017 20:52:16 +0100 Subject: [PATCH 3/9] Use auxiliary variable V807 Decreased performance. Consider creating a reference to avoid using the 'image.exifData()' expression repeatedly. crwimage.cpp 1320 --- src/crwimage_int.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/crwimage_int.cpp b/src/crwimage_int.cpp index fae860f1..8b8c611f 100644 --- a/src/crwimage_int.cpp +++ b/src/crwimage_int.cpp @@ -1137,10 +1137,11 @@ namespace Exiv2 { const ExifKey kX("Exif.Photo.PixelXDimension"); const ExifKey kY("Exif.Photo.PixelYDimension"); const ExifKey kO("Exif.Image.Orientation"); - const ExifData::const_iterator edX = image.exifData().findKey(kX); - const ExifData::const_iterator edY = image.exifData().findKey(kY); - const ExifData::const_iterator edO = image.exifData().findKey(kO); - const ExifData::const_iterator edEnd = image.exifData().end(); + const ExifData &exivData = image.exifData(); + const ExifData::const_iterator edX = exivData.findKey(kX); + const ExifData::const_iterator edY = exivData.findKey(kY); + const ExifData::const_iterator edO = exivData.findKey(kO); + const ExifData::const_iterator edEnd = exivData.end(); CiffComponent* cc = pHead->findComponent(pCrwMapping->crwTagId_, pCrwMapping->crwDir_); From 75cdbc8b91a82f3bb05c02adf8191e4d5f09b7e0 Mon Sep 17 00:00:00 2001 From: tbeu Date: Fri, 3 Nov 2017 20:57:24 +0100 Subject: [PATCH 4/9] Use pre-increment on iterators V803 Decreased performance. In case 'userEnd' is iterator it's more effective to use prefix form of increment. Replace iterator++ with ++iterator. futils.cpp 405 V803 Decreased performance. In case 'authEnd' is iterator it's more effective to use prefix form of increment. Replace iterator++ with ++iterator. futils.cpp 410 V803 Decreased performance. In case 'hostEnd' is iterator it's more effective to use prefix form of increment. Replace iterator++ with ++iterator. futils.cpp 428 V803 Decreased performance. In case 'e' is iterator it's more effective to use prefix form of increment. Replace iterator++ with ++iterator. epsimage.cpp 711 V803 Decreased performance. In case 'e' is iterator it's more effective to use prefix form of increment. Replace iterator++ with ++iterator. epsimage.cpp 841 V803 Decreased performance. In case 'e' is iterator it's more effective to use prefix form of increment. Replace iterator++ with ++iterator. epsimage.cpp 958 V803 Decreased performance. In case 'i' is iterator it's more effective to use prefix form of increment. Replace iterator++ with ++iterator. epsimage.cpp 855 V803 Decreased performance. In case 'it' is iterator it's more effective to use prefix form of increment. Replace iterator++ with ++iterator. jpgimage.cpp 817 V803 Decreased performance. In case 'lib' is iterator it's more effective to use prefix form of increment. Replace iterator++ with ++iterator. version.cpp 508 V803 Decreased performance. In case 'it' is iterator it's more effective to use prefix form of increment. Replace iterator++ with ++iterator. version.cpp 563 V803 Decreased performance. In case 'it' is iterator it's more effective to use prefix form of increment. Replace iterator++ with ++iterator. xmpsidecar.cpp 138 --- src/epsimage.cpp | 8 ++++---- src/futils.cpp | 6 +++--- src/jpgimage.cpp | 2 +- src/version.cpp | 4 ++-- src/xmpsidecar.cpp | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/epsimage.cpp b/src/epsimage.cpp index d70984ce..2ac06227 100644 --- a/src/epsimage.cpp +++ b/src/epsimage.cpp @@ -704,7 +704,7 @@ namespace { findXmp(posOtherXmp, sizeOtherXmp, data, posOtherXmp + sizeOtherXmp, posEndPageSetup, write); if (posOtherXmp >= posEndPageSetup) break; bool isRemovableEmbedding = false; - for (std::vector >::const_iterator e = removableEmbeddings.begin(); e != removableEmbeddings.end(); e++) { + for (std::vector >::const_iterator e = removableEmbeddings.begin(); e != removableEmbeddings.end(); ++e) { if (e->first <= posOtherXmp && posOtherXmp < e->second) { isRemovableEmbedding = true; break; @@ -834,7 +834,7 @@ namespace { if (useFlexibleEmbedding) { positions.push_back(xmpPos); } - for (std::vector >::const_iterator e = removableEmbeddings.begin(); e != removableEmbeddings.end(); e++) { + for (std::vector >::const_iterator e = removableEmbeddings.begin(); e != removableEmbeddings.end(); ++e) { positions.push_back(e->first); } std::sort(positions.begin(), positions.end()); @@ -848,7 +848,7 @@ namespace { const uint32_t posEpsNew = posTemp(*tempIo); size_t prevPos = posEps; size_t prevSkipPos = prevPos; - for (std::vector::const_iterator i = positions.begin(); i != positions.end(); i++) { + for (std::vector::const_iterator i = positions.begin(); i != positions.end(); ++i) { const size_t pos = *i; if (pos == prevPos) continue; #ifdef DEBUG @@ -951,7 +951,7 @@ namespace { } if (!useFlexibleEmbedding) { // remove preceding embedding(s) - for (std::vector >::const_iterator e = removableEmbeddings.begin(); e != removableEmbeddings.end(); e++) { + for (std::vector >::const_iterator e = removableEmbeddings.begin(); e != removableEmbeddings.end(); ++e) { if (pos == e->first) { skipPos = e->second; #ifdef DEBUG diff --git a/src/futils.cpp b/src/futils.cpp index 525c4185..0ea78113 100644 --- a/src/futils.cpp +++ b/src/futils.cpp @@ -411,12 +411,12 @@ namespace Exiv2 { iterator_t userEnd = std::find(authStart, authEnd, ':'); if (userEnd != authEnd) { result.Username = std::string(userStart, userEnd); - userEnd++; + ++userEnd; result.Password = std::string(userEnd, authEnd); } else { result.Username = std::string(authStart, authEnd); } - authEnd++; + ++authEnd; } else { authEnd = protocolEnd; } @@ -434,7 +434,7 @@ namespace Exiv2 { // port if ((hostEnd != uriEnd) && ((&*(hostEnd))[0] == ':')) // we have a port { - hostEnd++; + ++hostEnd; iterator_t portEnd = (pathStart != uriEnd) ? pathStart : queryStart; result.Port = std::string(hostEnd, portEnd); } diff --git a/src/jpgimage.cpp b/src/jpgimage.cpp index d4dd704d..7a181f1e 100644 --- a/src/jpgimage.cpp +++ b/src/jpgimage.cpp @@ -842,7 +842,7 @@ namespace Exiv2 { bool bOdd = (i % 2) != 0; bool bEven = !bOdd; pos[i + 1] = bEven ? *it : pos[i] + *it; - it++; + ++it; } pos[count + 1] = io_->size() - pos[count]; #ifdef DEBUG diff --git a/src/version.cpp b/src/version.cpp index 362c5b30..81a38f3d 100644 --- a/src/version.cpp +++ b/src/version.cpp @@ -479,7 +479,7 @@ void Exiv2::dumpLibraryInfo(std::ostream& os,const exv_grep_keys_t& keys) output(os,keys,"curl" , use_curl); if ( libs.begin() != libs.end() ) { output(os,keys,"executable" ,*libs.begin()); - for ( Exiv2::StringVector_i lib = libs.begin()+1 ; lib != libs.end() ; lib++ ) + for ( Exiv2::StringVector_i lib = libs.begin()+1 ; lib != libs.end() ; ++lib ) output(os,keys,"library",*lib); } @@ -530,7 +530,7 @@ void Exiv2::dumpLibraryInfo(std::ostream& os,const exv_grep_keys_t& keys) Exiv2::Dictionary ns; Exiv2::XmpProperties::registeredNamespaces(ns); - for ( Exiv2::Dictionary_i it = ns.begin(); it != ns.end() ; it++ ) { + for ( Exiv2::Dictionary_i it = ns.begin(); it != ns.end() ; ++it ) { std::string xmlns = (*it).first; std::string uri = (*it).second; output(os,keys,name,xmlns+":"+uri); diff --git a/src/xmpsidecar.cpp b/src/xmpsidecar.cpp index ba298640..de293e04 100644 --- a/src/xmpsidecar.cpp +++ b/src/xmpsidecar.cpp @@ -131,7 +131,7 @@ namespace Exiv2 { copyIptcToXmp(iptcData_, xmpData_); // #1112 - restore dates if they lost their TZ info - for ( Exiv2::Dictionary_i it = dates_.begin() ; it != dates_.end() ; it++) { + for ( Exiv2::Dictionary_i it = dates_.begin() ; it != dates_.end() ; ++it ) { std::string sKey = it->first; Exiv2::XmpKey key(sKey); if ( xmpData_.findKey(key) != xmpData_.end() ) { From 9569ef2fda8f0b4f9b1670e379ffd38d80b274dc Mon Sep 17 00:00:00 2001 From: tbeu Date: Fri, 3 Nov 2017 20:59:50 +0100 Subject: [PATCH 5/9] Use auxiliary variable V807 Decreased performance. Consider creating a reference to avoid using the 'image_.exifData()' expression repeatedly. preview.cpp 530 --- src/preview.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/preview.cpp b/src/preview.cpp index 572a9608..f12d3f15 100644 --- a/src/preview.cpp +++ b/src/preview.cpp @@ -524,22 +524,23 @@ namespace { : Loader(id, image) { offset_ = 0; - ExifData::const_iterator pos = image_.exifData().findKey(ExifKey(param_[parIdx].offsetKey_)); - if (pos != image_.exifData().end() && pos->count() > 0) { + const ExifData &exifData = image_.exifData(); + ExifData::const_iterator pos = exifData.findKey(ExifKey(param_[parIdx].offsetKey_)); + if (pos != exifData.end() && pos->count() > 0) { offset_ = pos->toLong(); } size_ = 0; - pos = image_.exifData().findKey(ExifKey(param_[parIdx].sizeKey_)); - if (pos != image_.exifData().end() && pos->count() > 0) { + pos = exifData.findKey(ExifKey(param_[parIdx].sizeKey_)); + if (pos != exifData.end() && pos->count() > 0) { size_ = pos->toLong(); } if (offset_ == 0 || size_ == 0) return; if (param_[parIdx].baseOffsetKey_) { - pos = image_.exifData().findKey(ExifKey(param_[parIdx].baseOffsetKey_)); - if (pos != image_.exifData().end() && pos->count() > 0) { + pos = exifData.findKey(ExifKey(param_[parIdx].baseOffsetKey_)); + if (pos != exifData.end() && pos->count() > 0) { offset_ += pos->toLong(); } } From 12d0da619b4bed604f7f10cb0c307e35805506b3 Mon Sep 17 00:00:00 2001 From: tbeu Date: Fri, 3 Nov 2017 21:01:28 +0100 Subject: [PATCH 6/9] Use clear to reset string V815 Decreased performance. Consider replacing the expression 'token = ""' with 'token.clear()'. http.cpp 193 --- src/http.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/http.cpp b/src/http.cpp index d63fff22..ce1801f3 100644 --- a/src/http.cpp +++ b/src/http.cpp @@ -187,7 +187,7 @@ static Exiv2::Dictionary stringToDict(const std::string& s) token += s[i]; } else { result[token]=token; - token=""; + token.clear(); } i++; } From bb9034e02916c30fe5b40f845a3904082d93c8f9 Mon Sep 17 00:00:00 2001 From: tbeu Date: Fri, 3 Nov 2017 21:09:44 +0100 Subject: [PATCH 7/9] Do not implicitly cast enum to Boolean V768 The expression 'fileProtocol(path)' is of enum type. It is odd that it is used as an expression of a Boolean-type. futils.cpp 288 --- src/futils.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/futils.cpp b/src/futils.cpp index 0ea78113..54066998 100644 --- a/src/futils.cpp +++ b/src/futils.cpp @@ -291,7 +291,7 @@ namespace Exiv2 { bool fileExists(const std::string& path, bool ct) { // special case: accept "-" (means stdin) - if (path.compare("-") == 0 || fileProtocol(path)) { + if (path.compare("-") == 0 || fileProtocol(path) != pFile) { return true; } @@ -306,7 +306,7 @@ namespace Exiv2 { bool fileExists(const std::wstring& wpath, bool ct) { // special case: accept "-" (means stdin) - if (wpath.compare(L"-") == 0 || fileProtocol(wpath)) { + if (wpath.compare(L"-") == 0 || fileProtocol(wpath) != pFile) { return true; } From eca251865f07ca1868cfc342a5a4cd4cb428abc3 Mon Sep 17 00:00:00 2001 From: tbeu Date: Fri, 3 Nov 2017 21:20:02 +0100 Subject: [PATCH 8/9] Fix check (on comparing unsigned minus signed greater zero) V555 The expression 'object->sizeDataArea_ - buf.size_ > 0' will work as 'object->sizeDataArea_ != buf.size_'. tiffvisitor.cpp 911 --- src/tiffvisitor_int.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tiffvisitor_int.cpp b/src/tiffvisitor_int.cpp index aa57ef27..4df1953e 100644 --- a/src/tiffvisitor_int.cpp +++ b/src/tiffvisitor_int.cpp @@ -905,7 +905,7 @@ namespace Exiv2 { #endif DataBuf buf = object->pValue()->dataArea(); memcpy(object->pDataArea_, buf.pData_, buf.size_); - if (object->sizeDataArea_ - buf.size_ > 0) { + if (object->sizeDataArea_ > static_cast(buf.size_)) { memset(object->pDataArea_ + buf.size_, 0x0, object->sizeDataArea_ - buf.size_); } From 6b1615840f8128474b3f36206cf1af7c2c71316c Mon Sep 17 00:00:00 2001 From: tbeu Date: Fri, 3 Nov 2017 21:24:53 +0100 Subject: [PATCH 9/9] Remove redundant check V547 Expression 'bPrint' is always true. rafimage.cpp 112 V547 Expression 'bPrint' is always true. rafimage.cpp 125 V547 Expression 'bPrint' is always true. rafimage.cpp 136 V547 Expression 'bPrint' is always true. rafimage.cpp 147 V547 Expression 'bPrint' is always true. rafimage.cpp 158 V547 Expression 'bPrint' is always true. rafimage.cpp 169 V547 Expression 'bPrint' is always true. rafimage.cpp 190 V547 Expression 'bPrint' is always true. rafimage.cpp 213 V547 Expression 'bPrint' is always true. rafimage.cpp 236 V547 Expression 'bPrint' is always true. rafimage.cpp 252 V547 Expression 'bPrint' is always true. rafimage.cpp 262 V547 Expression 'bPrint' is always true. rafimage.cpp 272 --- src/rafimage.cpp | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/rafimage.cpp b/src/rafimage.cpp index f709d544..7c305958 100644 --- a/src/rafimage.cpp +++ b/src/rafimage.cpp @@ -101,11 +101,11 @@ namespace Exiv2 { if (io_->error() || io_->eof()) throw Error(kerFailedToReadImageData); throw Error(kerNotAnImage, "RAF"); } - bool bPrint = option==kpsBasic || option==kpsRecursive; + const bool bPrint = option==kpsBasic || option==kpsRecursive; if ( bPrint ) { io_->seek(0,BasicIo::beg); // rewind - if ( bPrint ) { + { out << Internal::indent(depth) << "STRUCTURE OF RAF FILE: " << io().path() @@ -118,7 +118,7 @@ namespace Exiv2 { byte magicdata [17]; io_->read(magicdata, 16); magicdata[16] = 0; - if ( bPrint ) { + { out << Internal::indent(depth) << Internal::stringFormat(" %8u | %8u | ", 16, 0) << "Magic number : " @@ -129,7 +129,7 @@ namespace Exiv2 { byte data1 [5]; io_->read(data1, 4); data1[4] = 0; - if ( bPrint ) { + { out << Internal::indent(depth) << Internal::stringFormat(" %8u | %8u | ", 4, 16) << "data 1 : " @@ -140,7 +140,7 @@ namespace Exiv2 { byte data2 [9]; io_->read(data2, 8); data2[8] = 0; - if ( bPrint ) { + { out << Internal::indent(depth) << Internal::stringFormat(" %8u | %8u | ", 8, 20) << "data 2 : " @@ -151,7 +151,7 @@ namespace Exiv2 { byte camdata [33]; io_->read(camdata, 32); camdata[32] = 0; - if ( bPrint ) { + { out << Internal::indent(depth) << Internal::stringFormat(" %8u | %8u | ", 32, 28) << "camera : " @@ -162,7 +162,7 @@ namespace Exiv2 { byte dir_version [5]; io_->read(dir_version, 4); dir_version[4] = 0; - if ( bPrint ) { + { out << Internal::indent(depth) << Internal::stringFormat(" %8u | %8u | ", 4, 60) << "dir version : " @@ -183,7 +183,7 @@ namespace Exiv2 { std::stringstream j_len; j_off << jpg_img_off; j_len << jpg_img_len; - if ( bPrint ) { + { out << Internal::indent(depth) << Internal::stringFormat(" %8u | %8u | ", 4, 84) << "JPEG Image Offset : " @@ -206,7 +206,7 @@ namespace Exiv2 { std::stringstream ch_len; ch_off << cfa_hdr_off; ch_len << cfa_hdr_len; - if ( bPrint ) { + { out << Internal::indent(depth) << Internal::stringFormat(" %8u | %8u | ", 4, 92) << "CFA Header Offset : " @@ -229,7 +229,7 @@ namespace Exiv2 { std::stringstream c_len; c_off << cfa_off; c_len << cfa_len; - if ( bPrint ) { + { out << Internal::indent(depth) << Internal::stringFormat(" %8u | %8u | ", 4, 100) << "CFA Offset : " @@ -245,7 +245,7 @@ namespace Exiv2 { io_->seek(jpg_img_off, BasicIo::beg); // rewind DataBuf payload(16); // header is different from chunks io_->read(payload.pData_, payload.size_); - if ( bPrint ) { + { out << Internal::indent(depth) << Internal::stringFormat(" %8u | %8u | ", jpg_img_len, jpg_img_off) << "jpg image / exif : " @@ -255,7 +255,7 @@ namespace Exiv2 { io_->seek(cfa_hdr_off, BasicIo::beg); // rewind io_->read(payload.pData_, payload.size_); - if ( bPrint ) { + { out << Internal::indent(depth) << Internal::stringFormat(" %8u | %8u | ", cfa_hdr_len, cfa_hdr_off) << "CFA Header: " @@ -265,7 +265,7 @@ namespace Exiv2 { io_->seek(cfa_off, BasicIo::beg); // rewind io_->read(payload.pData_, payload.size_); - if ( bPrint ) { + { out << Internal::indent(depth) << Internal::stringFormat(" %8u | %8u | ", cfa_len, cfa_off) << "CFA : "