Merge pull request #2177 from neheb/1

clang-tidy and manual stuff

Author: piponazo

include/exiv2/error.hpp

@@ -262,7 +262,7 @@ class EXIV2API Error : public std::exception {
   }
 
   //! Virtual destructor. (Needed because of throw())
-  ~Error() noexcept override;
+  ~Error() noexcept override = default;
   //@}
 
   //! @name Accessors

samples/geotag.cpp

@@ -123,30 +123,30 @@ class Position {
   virtual ~Position() = default;
 
   //  instance methods
-  bool good() const {
+  [[nodiscard]] bool good() const {
     return time_ || lon_ || lat_ || ele_;
   }
   std::string getTimeString() {
     if (times_.empty())
       times_ = getExifTime(time_);
     return times_;
   }
-  time_t getTime() const {
+  [[nodiscard]] time_t getTime() const {
     return time_;
   }
-  std::string toString() const;
+  [[nodiscard]] std::string toString() const;
 
   //  getters/setters
-  double lat() const {
+  [[nodiscard]] double lat() const {
     return lat_;
   }
-  double lon() const {
+  [[nodiscard]] double lon() const {
     return lon_;
   }
-  double ele() const {
+  [[nodiscard]] double ele() const {
     return ele_;
   }
-  int delta() const {
+  [[nodiscard]] int delta() const {
     return delta_;
   }
   void delta(int delta) {

samples/ini-test.cpp

@@ -25,16 +25,15 @@ int main() {
   if (reader.ParseError() < 0) {
     std::cerr << "Can't load '" << ini << "'" << std::endl;
     return EXIT_FAILURE;
-  } else {
-    std::cout << "Config loaded from : '" << ini << "' "
-              << "version=" << reader.GetInteger("protocol", "version", -1)
-              << ", name=" << reader.Get("user", "name", "UNKNOWN")
-              << ", email=" << reader.Get("user", "email", "UNKNOWN") << ", pi=" << reader.GetReal("user", "pi", -1)
-              << ", active=" << reader.GetBoolean("user", "active", true) << std::endl;
-
-    std::cout << "169=" << reader.Get("canon", "169", "UNDEFINED")
-              << ", 170=" << reader.Get("canon", "170", "UNDEFINED") << std::endl;
   }
+  std::cout << "Config loaded from : '" << ini << "' "
+            << "version=" << reader.GetInteger("protocol", "version", -1)
+            << ", name=" << reader.Get("user", "name", "UNKNOWN")
+            << ", email=" << reader.Get("user", "email", "UNKNOWN") << ", pi=" << reader.GetReal("user", "pi", -1)
+            << ", active=" << reader.GetBoolean("user", "active", true) << std::endl;
+
+  std::cout << "169=" << reader.Get("canon", "169", "UNDEFINED") << ", 170=" << reader.Get("canon", "170", "UNDEFINED")
+            << std::endl;
 
   return EXIT_SUCCESS;
 }

src/basicio.cpp

@@ -619,19 +619,19 @@ class EXIV2API BlockMap {
     size_ = num;
   }
 
-  bool isNone() const {
+  [[nodiscard]] bool isNone() const {
     return type_ == bNone;
   }
 
-  bool isKnown() const {
+  [[nodiscard]] bool isKnown() const {
     return type_ == bKnown;
   }
 
-  byte* getData() const {
+  [[nodiscard]] byte* getData() const {
     return data_;
   }
 
-  size_t getSize() const {
+  [[nodiscard]] size_t getSize() const {
     return size_;
   }
 
@@ -1525,7 +1525,7 @@ HttpIo::HttpIo(const std::string& url, size_t blockSize) {
 class CurlIo::CurlImpl : public Impl {
  public:
   //! Constructor
-  CurlImpl(const std::string& path, size_t blockSize);
+  CurlImpl(const std::string& url, size_t blockSize);
   //! Destructor. Cleans up the curl pointer and releases all managed memory.
   ~CurlImpl() override;
 

src/bmffimage.cpp

@@ -527,7 +527,7 @@ void BmffImage::parseXmp(uint64_t length, uint64_t start) {
     long restore = io_->tell();
     io_->seek(static_cast<long>(start), BasicIo::beg);
 
-    size_t lengthSizeT = static_cast<size_t>(length);
+    auto lengthSizeT = static_cast<size_t>(length);
     DataBuf xmp(lengthSizeT + 1);
     xmp.write_uint8(lengthSizeT, 0);  // ensure xmp is null terminated!
     if (io_->read(xmp.data(), lengthSizeT) != lengthSizeT)

src/convert.cpp

@@ -26,8 +26,8 @@
 #endif
 
 #ifdef EXV_HAVE_ICONV
-#include <errno.h>
 #include <iconv.h>
+#include <cerrno>
 #endif
 
 // Adobe XMP Toolkit
@@ -262,7 +262,7 @@ class Converter {
   //! @name Accessors
   //@{
   //! Get the value of the erase flag, see also setErase(bool on).
-  bool erase() const {
+  [[nodiscard]] bool erase() const {
     return erase_;
   }
   //@}
@@ -1527,7 +1527,7 @@ bool asciiToUtf8(std::string& /*str*/) {
   return true;
 }
 
-using ConvFct = bool (*)(std::string& str);
+using ConvFct = std::function<bool(std::string&)>;
 
 struct ConvFctList {
   bool operator==(const std::pair<const char*, const char*>& fromTo) const {

src/cr2image.cpp

@@ -17,7 +17,6 @@
 // *****************************************************************************
 // class member definitions
 namespace Exiv2 {
-using namespace Internal;
 
 Cr2Image::Cr2Image(BasicIo::UniquePtr io, bool /*create*/) :
     Image(ImageType::cr2, mdExif | mdIptc | mdXmp, std::move(io)) {
@@ -87,7 +86,7 @@ void Cr2Image::writeMetadata() {
     if (isCr2Type(*io_, false)) {
       pData = io_->mmap(true);
       size = io_->size();
-      Cr2Header cr2Header;
+      Internal::Cr2Header cr2Header;
       if (0 == cr2Header.read(pData, 16)) {
         bo = cr2Header.byteOrder();
       }
@@ -101,9 +100,9 @@ void Cr2Image::writeMetadata() {
 }  // Cr2Image::writeMetadata
 
 ByteOrder Cr2Parser::decode(ExifData& exifData, IptcData& iptcData, XmpData& xmpData, const byte* pData, size_t size) {
-  Cr2Header cr2Header;
-  return TiffParserWorker::decode(exifData, iptcData, xmpData, pData, size, Tag::root, TiffMapping::findDecoder,
-                                  &cr2Header);
+  Internal::Cr2Header cr2Header;
+  return Internal::TiffParserWorker::decode(exifData, iptcData, xmpData, pData, size, Internal::Tag::root,
+                                            Internal::TiffMapping::findDecoder, &cr2Header);
 }
 
 WriteMethod Cr2Parser::encode(BasicIo& io, const byte* pData, size_t size, ByteOrder byteOrder,
@@ -113,20 +112,20 @@ WriteMethod Cr2Parser::encode(BasicIo& io, const byte* pData, size_t size, ByteO
 
   // Delete IFDs which do not occur in TIFF images
   static constexpr auto filteredIfds = std::array{
-      panaRawId,
+      Internal::panaRawId,
   };
   for (auto&& filteredIfd : filteredIfds) {
 #ifdef EXIV2_DEBUG_MESSAGES
     std::cerr << "Warning: Exif IFD " << filteredIfd << " not encoded\n";
 #endif
-    ed.erase(std::remove_if(ed.begin(), ed.end(), FindExifdatum(filteredIfd)), ed.end());
+    ed.erase(std::remove_if(ed.begin(), ed.end(), Internal::FindExifdatum(filteredIfd)), ed.end());
   }
 
-  Cr2Header header(byteOrder);
-  OffsetWriter offsetWriter;
-  offsetWriter.setOrigin(OffsetWriter::cr2RawIfdOffset, Cr2Header::offset2addr(), byteOrder);
-  return TiffParserWorker::encode(io, pData, size, ed, iptcData, xmpData, Tag::root, TiffMapping::findEncoder, &header,
-                                  &offsetWriter);
+  auto header = Internal::Cr2Header(byteOrder);
+  Internal::OffsetWriter offsetWriter;
+  offsetWriter.setOrigin(Internal::OffsetWriter::cr2RawIfdOffset, Internal::Cr2Header::offset2addr(), byteOrder);
+  return Internal::TiffParserWorker::encode(io, pData, size, ed, iptcData, xmpData, Internal::Tag::root,
+                                            Internal::TiffMapping::findEncoder, &header, &offsetWriter);
 }
 
 // *************************************************************************
@@ -146,7 +145,7 @@ bool isCr2Type(BasicIo& iIo, bool advance) {
   if (iIo.error() || iIo.eof()) {
     return false;
   }
-  Cr2Header header;
+  Internal::Cr2Header header;
   bool rc = header.read(buf, len);
   if (!advance || !rc) {
     iIo.seek(-len, BasicIo::cur);

src/crwimage.cpp

@@ -18,7 +18,6 @@
 // *****************************************************************************
 // class member definitions
 namespace Exiv2 {
-using namespace Internal;
 
 CrwImage::CrwImage(BasicIo::UniquePtr io, bool /*create*/) : Image(ImageType::crw, mdExif | mdComment, std::move(io)) {
 }  // CrwImage::CrwImage
@@ -102,12 +101,12 @@ void CrwImage::writeMetadata() {
 
 void CrwParser::decode(CrwImage* pCrwImage, const byte* pData, size_t size) {
   // Parse the image, starting with a CIFF header component
-  CiffHeader header;
+  Internal::CiffHeader header;
   header.read(pData, size);
   header.decode(*pCrwImage);
 
   // a hack to get absolute offset of preview image inside CRW structure
-  CiffComponent* preview = header.findComponent(0x2007, 0x0000);
+  auto preview = header.findComponent(0x2007, 0x0000);
   if (preview) {
     (pCrwImage->exifData())["Exif.Image2.JPEGInterchangeFormat"] = uint32_t(preview->pData() - pData);
     (pCrwImage->exifData())["Exif.Image2.JPEGInterchangeFormatLength"] = static_cast<uint32_t>(preview->size());
@@ -116,14 +115,14 @@ void CrwParser::decode(CrwImage* pCrwImage, const byte* pData, size_t size) {
 
 void CrwParser::encode(Blob& blob, const byte* pData, size_t size, const CrwImage* pCrwImage) {
   // Parse image, starting with a CIFF header component
-  CiffHeader header;
+  Internal::CiffHeader header;
   if (size != 0) {
     header.read(pData, size);
   }
 
   // Encode Exif tags from image into the CRW parse tree and write the
   // structure to the binary image blob
-  CrwMap::encode(&header, *pCrwImage);
+  Internal::CrwMap::encode(&header, *pCrwImage);
   header.write(blob);
 }
 
@@ -147,7 +146,7 @@ bool isCrwType(BasicIo& iIo, bool advance) {
   if (!(('I' == tmpBuf[0] && 'I' == tmpBuf[1]) || ('M' == tmpBuf[0] && 'M' == tmpBuf[1]))) {
     result = false;
   }
-  if (result && std::memcmp(tmpBuf + 6, CiffHeader::signature(), 8) != 0) {
+  if (result && std::memcmp(tmpBuf + 6, Internal::CiffHeader::signature(), 8) != 0) {
     result = false;
   }
   if (!advance || !result)

src/crwimage_int.cpp

@@ -22,10 +22,7 @@ class RotationMap {
 
  private:
   //! Helper structure for the mapping list
-  struct OmList {
-    uint16_t orientation;  //!< Exif orientation value
-    int32_t degrees;       //!< CRW Rotation degrees
-  };
+  using OmList = std::pair<uint16_t, int32_t>;
   // DATA
   static const OmList omList_[];
 };  // class RotationMap
@@ -350,7 +347,7 @@ size_t CiffDirectory::doWrite(Blob& blob, ByteOrder byteOrder, size_t offset) {
   for (auto&& component : components_) {
     dirOffset = component->write(blob, byteOrder, dirOffset);
   }
-  const uint32_t dirStart = static_cast<uint32_t>(dirOffset);
+  const auto dirStart = static_cast<uint32_t>(dirOffset);
 
   // Number of directory entries
   byte buf[4];
@@ -520,7 +517,7 @@ CiffComponent* CiffDirectory::doFindComponent(uint16_t crwTagId, uint16_t crwDir
 void CiffHeader::add(uint16_t crwTagId, uint16_t crwDir, DataBuf&& buf) {
   CrwDirs crwDirs;
   CrwMap::loadStack(crwDirs, crwDir);
-  [[maybe_unused]] uint16_t rootDirectory = crwDirs.top().crwDir_;
+  [[maybe_unused]] auto [rootDirectory, _] = crwDirs.top();
   crwDirs.pop();
   if (!pRootDir_) {
     pRootDir_ = std::make_unique<CiffDirectory>();
@@ -553,18 +550,18 @@ CiffComponent* CiffDirectory::doAdd(CrwDirs& crwDirs, uint16_t crwTagId) {
         set value
   */
   if (!crwDirs.empty()) {
-    CrwSubDir csd = crwDirs.top();
+    auto [dir, parent] = crwDirs.top();
     crwDirs.pop();
     // Find the directory
     for (auto&& component : components_) {
-      if (component->tag() == csd.crwDir_) {
+      if (component->tag() == dir) {
         cc_ = component;
         break;
       }
     }
     if (!cc_) {
       // Directory doesn't exist yet, add it
-      m_ = std::make_unique<CiffDirectory>(csd.crwDir_, csd.parent_);
+      m_ = std::make_unique<CiffDirectory>(dir, parent);
       cc_ = m_.get();
       add(std::move(m_));
     }
@@ -592,7 +589,6 @@ void CiffHeader::remove(uint16_t crwTagId, uint16_t crwDir) {
   if (pRootDir_) {
     CrwDirs crwDirs;
     CrwMap::loadStack(crwDirs, crwDir);
-    [[maybe_unused]] uint16_t rootDirectory = crwDirs.top().crwDir_;
     crwDirs.pop();
     pRootDir_->remove(crwDirs, crwTagId);
   }
@@ -608,11 +604,11 @@ void CiffComponent::doRemove(CrwDirs& /*crwDirs*/, uint16_t /*crwTagId*/) {
 
 void CiffDirectory::doRemove(CrwDirs& crwDirs, uint16_t crwTagId) {
   if (!crwDirs.empty()) {
-    CrwSubDir csd = crwDirs.top();
+    auto [dir, _] = crwDirs.top();
     crwDirs.pop();
     // Find the directory
     for (auto i = components_.begin(); i != components_.end(); ++i) {
-      if ((*i)->tag() == csd.crwDir_) {
+      if ((*i)->tag() == dir) {
         // Recursive call to next lower level directory
         (*i)->remove(crwDirs, crwTagId);
         if ((*i)->empty())
@@ -743,8 +739,8 @@ void CrwMap::decodeArray(const CiffComponent& ciffComponent, const CrwMapping* p
   if (ifdId == canonSiId) {
     // Exif.Photo.FNumber
     float f = fnumber(canonEv(aperture));
-    Rational r = floatToRationalCast(f);
-    URational ur(r.first, r.second);
+    auto [r, s] = floatToRationalCast(f);
+    auto ur = URational(r, s);
     URationalValue fn;
     fn.value_.push_back(ur);
     image.exifData().add(ExifKey("Exif.Photo.FNumber"), &fn);
@@ -835,9 +831,10 @@ void CrwMap::decodeBasic(const CiffComponent& ciffComponent, const CrwMapping* p
 
 void CrwMap::loadStack(CrwDirs& crwDirs, uint16_t crwDir) {
   for (auto&& crw : crwSubDir_) {
-    if (crw.crwDir_ == crwDir) {
+    auto&& [dir, parent] = crw;
+    if (dir == crwDir) {
       crwDirs.push(crw);
-      crwDir = crw.parent_;
+      crwDir = parent;
     }
   }
 }  // CrwMap::loadStack

src/crwimage_int.hpp

@@ -20,7 +20,7 @@ namespace Exiv2::Internal {
 class CiffHeader;
 class CiffComponent;
 struct CrwMapping;
-struct CrwSubDir;
+using CrwSubDir = std::pair<uint16_t, uint16_t>;
 
 // *****************************************************************************
 // type definitions
@@ -486,12 +486,6 @@ class CiffHeader {
 
 };  // class CiffHeader
 
-//! Structure for the CIFF directory hierarchy
-struct CrwSubDir {
-  uint16_t crwDir_;  //!< Directory tag
-  uint16_t parent_;  //!< Parent directory tag
-};                   // struct CrwSubDir
-
 /*!
   @brief Structure for a mapping table for conversion of CIFF entries to
          image metadata and vice versa.