Merge pull request #2060 from Exiv2/main_bmpTests

Extend unit tests for BMP & Datasets

.github/workflows/on_PR_linux_special_builds.yml

@@ -17,6 +17,7 @@ jobs:
         run: |
           sudo apt-get install ninja-build
           pip3 install conan==1.43.0
+          pip3 install gcovr
 
       - name: Conan common config
         run: |
@@ -32,24 +33,36 @@ jobs:
 
       - name: Build
         run: |
-          cd build
-          cmake -GNinja -DCMAKE_BUILD_TYPE=Debug -DBUILD_SHARED_LIBS=ON -DEXIV2_ENABLE_PNG=ON -DEXIV2_ENABLE_WEBREADY=ON -DEXIV2_ENABLE_CURL=ON -DEXIV2_BUILD_UNIT_TESTS=ON -DEXIV2_ENABLE_BMFF=ON -DEXIV2_TEAM_WARNINGS_AS_ERRORS=ON -DBUILD_WITH_COVERAGE=ON -DCMAKE_INSTALL_PREFIX=install ..
+          cd build && \
+          cmake -GNinja \
+            -DCMAKE_BUILD_TYPE=Debug \
+            -DBUILD_SHARED_LIBS=ON \
+            -DEXIV2_ENABLE_PNG=ON \
+            -DEXIV2_ENABLE_WEBREADY=ON \
+            -DEXIV2_ENABLE_CURL=ON \
+            -DEXIV2_BUILD_UNIT_TESTS=ON \
+            -DEXIV2_ENABLE_BMFF=ON \
+            -DEXIV2_TEAM_WARNINGS_AS_ERRORS=ON \
+            -DEXIV2_BUILD_SAMPLES=ON \
+            -DBUILD_WITH_COVERAGE=ON \
+            -DCMAKE_INSTALL_PREFIX=install \
+            .. && \
           cmake --build .
 
       - name: Tests + Upload coverage
         run: |
           cd build
           ctest --output-on-failure
-          pip install gcovr
-          gcovr -r ./../ -x --exclude-unreachable-branches --exclude-throw-branches -o coverage.xml .
+          gcovr --root .. --object-dir . --exclude-directories xmpsdk --exclude-directories unitTests --exclude-directories samples --exclude '.*xmpsdk.*' --exclude '.*unitTests.*' --exclude '.*samples.*' --exclude-unreachable-branches --exclude-throw-branches --xml -o coverage.xml
           curl https://keybase.io/codecovsecurity/pgp_keys.asc | gpg --import
           curl -Os https://uploader.codecov.io/latest/linux/codecov
           curl -Os https://uploader.codecov.io/latest/linux/codecov.SHA256SUM
           curl -Os https://uploader.codecov.io/latest/linux/codecov.SHA256SUM.sig
           gpg --verify codecov.SHA256SUM.sig codecov.SHA256SUM
           shasum -a 256 -c codecov.SHA256SUM
           chmod +x codecov
-          ./codecov -f build/coverage.xml
+          ls -lh
+          ./codecov -f coverage.xml
 
   special_releaseValgrind:
     name: 'Ubuntu 20.04 - GCC - Release+Valgrind'

cmake/gcovr.cmake

@@ -14,6 +14,7 @@ if(BUILD_WITH_COVERAGE)
             COMMAND ${GCOVR} --root ${CMAKE_SOURCE_DIR} --object-dir=${CMAKE_BINARY_DIR} --html --html-details -o coverage_output/coverage.html
               --exclude-directories xmpsdk --exclude-directories unitTests --exclude-directories samples
               --exclude '.*xmpsdk.*' --exclude '.*unitTests.*' --exclude '.*samples.*'
+              --exclude-unreachable-branches --exclude-throw-branches
 
             WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
         )

include/exiv2/bmpimage.hpp

@@ -79,33 +79,24 @@ namespace Exiv2 {
         //! @name Manipulators
         //@{
         void readMetadata() override;
-        /*!
-          @brief Todo: Write metadata back to the image. This method is not
-              yet(?) implemented. Calling it will throw an Error(kerWritingImageFormatUnsupported).
-         */
+
+        /// @throws Error(kerWritingImageFormatUnsupported).
         void writeMetadata() override;
-        /*!
-          @brief Todo: Not supported yet(?). Calling this function will throw
-              an instance of Error(kerInvalidSettingForImage).
-         */
+
+        /// @throws Error(kerInvalidSettingForImage)
         void setExifData(const ExifData& exifData) override;
-        /*!
-          @brief Todo: Not supported yet(?). Calling this function will throw
-              an instance of Error(kerInvalidSettingForImage).
-         */
+
+        /// @throws Error(kerInvalidSettingForImage)
         void setIptcData(const IptcData& iptcData) override;
-        /*!
-          @brief Not supported. Calling this function will throw an instance
-              of Error(kerInvalidSettingForImage).
-         */
+
+        /// @throws Error(kerInvalidSettingForImage)
         void setComment(const std::string& comment) override;
         //@}
 
         //! @name Accessors
         //@{
         std::string mimeType() const override;
         //@}
-
     }; // class BmpImage
 
 // *****************************************************************************

include/exiv2/datasets.hpp

@@ -97,6 +97,7 @@ namespace Exiv2 {
         static constexpr uint16_t UNO                    = 100;
         static constexpr uint16_t ARMId                  = 120;
         static constexpr uint16_t ARMVersion             = 122;
+
         static constexpr uint16_t RecordVersion          = 0;
         static constexpr uint16_t ObjectType             = 3;
         static constexpr uint16_t ObjectAttribute        = 4;
@@ -172,20 +173,23 @@ namespace Exiv2 {
                   dataset.
          */
         static std::string dataSetName(uint16_t number, uint16_t recordId);
+
         /*!
           @brief Return the title (label) of the dataset.
           @param number The dataset number
           @param recordId The IPTC record Id
           @return The title (label) of the dataset
          */
         static const char* dataSetTitle(uint16_t number, uint16_t recordId);
+
         /*!
           @brief Return the description of the dataset.
           @param number The dataset number
           @param recordId The IPTC record Id
           @return The description of the dataset
          */
         static const char* dataSetDesc(uint16_t number, uint16_t recordId);
+
         /*!
           @brief Return the Photoshop name of a given dataset.
           @param number The dataset number
@@ -194,13 +198,15 @@ namespace Exiv2 {
                  string if Photoshop does not use the dataset.
          */
         static const char* dataSetPsName(uint16_t number, uint16_t recordId);
+
         /*!
           @brief Check if a given dataset is repeatable
           @param number The dataset number
           @param recordId The IPTC record Id
           @return true if the given dataset is repeatable otherwise false
          */
         static bool dataSetRepeatable(uint16_t number, uint16_t recordId);
+
         /*!
           @brief Return the dataSet number for dataset name and record id
 
@@ -212,8 +218,10 @@ namespace Exiv2 {
           @throw Error if the \em dataSetName or \em recordId are invalid
          */
         static uint16_t dataSet(const std::string& dataSetName, uint16_t recordId);
+
         //! Return the type for dataSet number and Record id
         static TypeId dataSetType(uint16_t number, uint16_t recordId);
+
         /*!
           @brief Return the name of the Record
           @param recordId The record id
@@ -222,32 +230,21 @@ namespace Exiv2 {
                   unknown record.
          */
         static std::string recordName(uint16_t recordId);
+
         /*!
            @brief Return the description of a record
            @param recordId Record Id number
            @return the description of the Record
          */
         static const char* recordDesc(uint16_t recordId);
-        /*!
-           @brief Return the Id number of a record
-           @param recordName Name of a record type
-           @return the Id number of a Record
-           @throw Error if the record is not known;
-         */
-        static uint16_t recordId(const std::string& recordName);
-        //! Return read-only list of built-in Envelope Record datasets
-        static const DataSet* envelopeRecordList();
-        //! Return read-only list of built-in Application2 Record datasets
-        static const DataSet* application2RecordList();
-        //! Print a list of all dataSets to output stream
+
         static void dataSetList(std::ostream& os);
 
     private:
         static int dataSetIdx(uint16_t number, uint16_t recordId);
         static int dataSetIdx(const std::string& dataSetName, uint16_t recordId);
 
         static const DataSet* const records_[];
-        static const RecordInfo recordInfo_[];
 
     }; // class IptcDataSets
 
@@ -278,17 +275,11 @@ namespace Exiv2 {
         IptcKey(uint16_t tag, uint16_t record);
         //! Copy constructor
         IptcKey(const IptcKey& rhs);
+        IptcKey& operator=(const IptcKey& rhs) = delete;
         //! Destructor
         ~IptcKey() override = default;
         //@}
 
-        //! @name Manipulators
-        //@{
-        /*!
-          @brief Assignment operator.
-         */
-        IptcKey& operator=(const IptcKey& rhs);
-        //@}
 
         //! @name Accessors
         //@{
@@ -331,10 +322,6 @@ namespace Exiv2 {
         //! Internal virtual copy constructor.
         IptcKey* clone_() const override;
 
-        // DATA
-#ifndef SWIG
-        static constexpr auto familyName_ = "Iptc";
-#endif
         uint16_t tag_;                 //!< Tag value
         uint16_t record_;              //!< Record value
         std::string key_;              //!< Key

include/exiv2/image.hpp

@@ -368,9 +368,8 @@ namespace Exiv2 {
                  encoded. Initially, it is not set (\em invalidByteOrder).
          */
         ByteOrder byteOrder() const;
-        /*!
-          @brief Check if the Image instance is valid. Use after object
-              construction.
+
+        /*! @brief Check if the Image instance is valid. Use after object construction.
           @return true if the Image is in a valid state.
          */
         bool good() const;

include/exiv2/iptc.hpp

@@ -45,6 +45,8 @@ namespace Exiv2 {
     /*!
       @brief An IPTC metadatum ("dataset"), consisting of an IptcKey and a
              Value and methods to manipulate these.
+
+             This is referred in the standard as a property.
      */
     class EXIV2API Iptcdatum : public Metadatum {
     public:
@@ -156,8 +158,7 @@ namespace Exiv2 {
     typedef std::vector<Iptcdatum> IptcMetadata;
 
     /*!
-      @brief A container for IPTC data. This is a top-level class of
-             the %Exiv2 library.
+      @brief A container for IPTC data. This is a top-level class of the %Exiv2 library.
 
       Provide high-level access to the IPTC data of an image:
       - read IPTC information from JPEG files

src/bmpimage.cpp

@@ -47,24 +47,22 @@ namespace Exiv2
 
     std::string BmpImage::mimeType() const
     {
-        return "image/x-ms-bmp";
+        // "image/bmp" is a Generic Bitmap
+        return "image/x-ms-bmp"; // Microsoft Bitmap
     }
 
     void BmpImage::setExifData(const ExifData& /*exifData*/)
     {
-        // Todo: implement me!
         throw(Error(kerInvalidSettingForImage, "Exif metadata", "BMP"));
     }
 
     void BmpImage::setIptcData(const IptcData& /*iptcData*/)
     {
-        // Todo: implement me!
         throw(Error(kerInvalidSettingForImage, "IPTC metadata", "BMP"));
     }
 
     void BmpImage::setComment(const std::string& /*comment*/)
     {
-        // not supported
         throw(Error(kerInvalidSettingForImage, "Image comment", "BMP"));
     }
 
@@ -77,9 +75,11 @@ namespace Exiv2
             throw Error(kerDataSourceOpenFailed, io_->path(), strError());
         }
         IoCloser closer(*io_);
+
         // Ensure that this is the correct image type
         if (!isBmpType(*io_, false)) {
-            if (io_->error() || io_->eof()) throw Error(kerFailedToReadImageData);
+            if (io_->error() || io_->eof())
+                throw Error(kerFailedToReadImageData);
             throw Error(kerNotAnImage, "BMP");
         }
         clearMetadata();
@@ -105,7 +105,7 @@ namespace Exiv2
           46      4 bytes  color count
           50      4 bytes  important colors       number of "important" colors
         */
-        byte buf[54];
+        byte buf[26];
         if (io_->read(buf, sizeof(buf)) == sizeof(buf)) {
             pixelWidth_ = getLong(buf + 18, littleEndian);
             pixelHeight_ = getLong(buf + 22, littleEndian);
@@ -114,7 +114,7 @@ namespace Exiv2
 
     void BmpImage::writeMetadata()
     {
-        // Todo: implement me!
+        /// \todo implement me!
         throw(Error(kerWritingImageFormatUnsupported, "BMP"));
     }