Revisiting cmake code for generating coverage reports

.gitignore

@@ -23,4 +23,5 @@ src/doxygen.hpp
 test/tmp/*
 doc/html
 contrib/vms/.vagrant
-/.vscode
+/.vscode
+.vs/

cmake/gcovr.cmake

@@ -1,8 +1,8 @@
-# Intentd usage
-#   cmake -DBUILD_WITH_COVERAGE=yes ../
-#   make -j
-#   make tests
-#   make coverage
+# Intended usage
+#   cmake -DCMAKE_BUILD_TYPE=Debug -DBUILD_WITH_COVERAGE=yes ../
+#   cmake --build . --config Debug 
+#   ctest
+#   cmake --build . --config Debug --target coverage
 
 if(BUILD_WITH_COVERAGE)
     find_program (GCOVR gcovr)
@@ -12,6 +12,9 @@ if(BUILD_WITH_COVERAGE)
         add_custom_command(OUTPUT _run_gcovr_parser
             POST_BUILD
             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.*'
+
             WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
         )
         add_custom_target (coverage DEPENDS _run_gcovr_parser)

include/exiv2/basicio.hpp

@@ -183,11 +183,8 @@ namespace Exiv2 {
           @return 0 if successful;<BR>
               Nonzero if failure;
          */
-#if defined(_MSC_VER)
         virtual int seek(int64_t offset, Position pos) = 0;
-#else
-        virtual int seek(long offset, Position pos) = 0;
-#endif
+
         /*!
           @brief Safe version of `seek()` that checks for errors and throws
               an exception if the seek was unsuccessful.
@@ -196,11 +193,7 @@ namespace Exiv2 {
           @param pos Position from which the seek should start
           @param err Error code to use if an exception is thrown.
          */
-#if defined(_MSC_VER)
         void seekOrThrow(int64_t offset, Position pos, ErrorCode err);
-#else
-        void seekOrThrow(long offset, Position pos, ErrorCode err);
-#endif
 
         /*!
           @brief Direct access to the IO data. For files, this is done by
@@ -263,7 +256,7 @@ namespace Exiv2 {
           @note This method should be only called after the concerned data (metadata)
                 are all downloaded from the remote file to memory.
          */
-        virtual void populateFakeData() {}
+        virtual void populateFakeData() = 0;
 
         /*!
           @brief this is allocated and populated by mmap()
@@ -442,19 +435,9 @@ namespace Exiv2 {
           @throw Error In case of failure
          */
         void transfer(BasicIo& src) override;
-        /*!
-          @brief Move the current file position.
-          @param offset Number of bytes to move the file position
-              relative to the starting position specified by \em pos
-          @param pos Position from which the seek should start
-          @return 0 if successful;<BR>
-                 Nonzero if failure;
-         */
-#if defined(_MSC_VER)
+
         int seek(int64_t offset, Position pos) override;
-#else
-        int seek(long offset, Position pos) override;
-#endif
+
         /*!
           @brief Map the file into the process's address space. The file must be
                  open before mmap() is called. If the mapped area is writeable,
@@ -664,19 +647,9 @@ namespace Exiv2 {
           @throw Error In case of failure
          */
         void transfer(BasicIo& src) override;
-        /*!
-          @brief Move the current IO position.
-          @param offset Number of bytes to move the IO position
-              relative to the starting position specified by \em pos
-          @param pos Position from which the seek should start
-          @return 0 if successful;<BR>
-                 Nonzero if failure;
-         */
-#if defined(_MSC_VER)
-        virtual int seek(int64_t offset, Position pos);
-#else
-        int seek(long offset, Position pos) override;
-#endif
+
+        int seek(int64_t offset, Position pos) override;
+
         /*!
           @brief Allow direct access to the underlying data buffer. The buffer
                  is not protected against write access in any way, the argument
@@ -949,19 +922,9 @@ namespace Exiv2 {
           @note The write access is only supported by http, https, ssh.
          */
         void transfer(BasicIo& src) override;
-        /*!
-          @brief Move the current IO position.
-          @param offset Number of bytes to move the IO position
-              relative to the starting position specified by \em pos
-          @param pos Position from which the seek should start
-          @return 0 if successful;<BR>
-                 Nonzero if failure;
-         */
-#if defined(_MSC_VER)
-       virtual int seek(int64_t offset, Position pos);
-#else
-        int seek(long offset, Position pos) override;
-#endif
+
+       int seek(int64_t offset, Position pos) override;
+
        /*!
          @brief Not support
          @return NULL

src/basicio.cpp

@@ -84,11 +84,7 @@ namespace Exiv2 {
         enforce(!error(), err);
     }
 
-#if defined(_MSC_VER)
     void BasicIo::seekOrThrow(int64_t offset, Position pos, ErrorCode err) {
-#else
-    void BasicIo::seekOrThrow(long offset, Position pos, ErrorCode err) {
-#endif
         const int r = seek(offset, pos);
         enforce(r == 0, err);
     }
@@ -628,6 +624,7 @@ namespace Exiv2 {
             fileIo->close();
             // Check if the file can be written to, if it already exists
             if (open("a+b") != 0) {
+                /// \todo Use std::filesystem once C++17 can be used
                 // Remove the (temporary) file
 #ifdef EXV_UNICODE_PATH
                 if (fileIo->p_->wpMode_ == Impl::wpUnicode) {
@@ -898,7 +895,6 @@ namespace Exiv2 {
         return putc(data, p_->fp_);
     }
 
-#if defined(_MSC_VER)
     int FileIo::seek( int64_t offset, Position pos )
     {
         assert(p_->fp_ != 0);
@@ -917,24 +913,6 @@ namespace Exiv2 {
         return std::fseek(p_->fp_,static_cast<long>(offset), fileSeek);
 #endif
     }
-#else
-    int FileIo::seek(long offset, Position pos)
-    {
-        assert(p_->fp_ != 0);
-
-        int fileSeek = 0;
-        switch (pos) {
-        case BasicIo::cur: fileSeek = SEEK_CUR; break;
-        case BasicIo::beg: fileSeek = SEEK_SET; break;
-        case BasicIo::end: fileSeek = SEEK_END; break;
-        }
-
-        if (p_->switchMode(Impl::opSeek) != 0) {
-            return 1;
-        }
-        return std::fseek(p_->fp_, offset, fileSeek);
-    }
-#endif
 
     long FileIo::tell() const
     {
@@ -1279,7 +1257,6 @@ namespace Exiv2 {
         return data;
     }
 
-#if defined(_MSC_VER)
     int MemIo::seek( int64_t offset, Position pos )
     {
         int64_t newIdx = 0;
@@ -1302,30 +1279,6 @@ namespace Exiv2 {
         p_->eof_ = false;
         return 0;
     }
-#else
-    int MemIo::seek(long offset, Position pos)
-    {
-        long newIdx = 0;
-
-        switch (pos) {
-        case BasicIo::cur: newIdx = p_->idx_ + offset; break;
-        case BasicIo::beg: newIdx = offset; break;
-        case BasicIo::end: newIdx = p_->size_ + offset; break;
-        }
-
-        if (newIdx < 0)
-            return 1;
-
-        if (newIdx > p_->size_) {
-            p_->eof_ = true;
-            return 1;
-        }
-
-        p_->idx_ = newIdx;
-        p_->eof_ = false;
-        return 0;
-    }
-#endif
 
     byte* MemIo::mmap(bool /*isWriteable*/)
     {
@@ -1368,22 +1321,21 @@ namespace Exiv2 {
     {
         DataBuf buf(rcount);
         long readCount = read(buf.data(), buf.size());
-        if (readCount < 0) {
-            throw Error(kerInputDataReadFailed);
-        }
         buf.resize(readCount);
         return buf;
     }
 
     long MemIo::read(byte* buf, long rcount)
     {
-        long avail = std::max(p_->size_ - p_->idx_, 0L);
-        long allow = std::min(rcount, avail);
+        const long avail = std::max(p_->size_ - p_->idx_, 0L);
+        const long allow = std::min(rcount, avail);
         if (allow > 0) {
             std::memcpy(buf, &p_->data_[p_->idx_], allow);
         }
         p_->idx_ += allow;
-        if (rcount > avail) p_->eof_ = true;
+        if (rcount > avail) {
+          p_->eof_ = true;
+        }
         return allow;
     }
 
@@ -1919,28 +1871,10 @@ namespace Exiv2 {
         src.close();
     }
 
-#if defined(_MSC_VER)
-    int RemoteIo::seek( int64_t offset, Position pos )
+    int RemoteIo::seek( int64_t offset, Position pos)
     {
         assert(p_->isMalloced_);
-        uint64_t newIdx = 0;
-
-        switch (pos) {
-            case BasicIo::cur: newIdx = p_->idx_ + offset; break;
-            case BasicIo::beg: newIdx = offset; break;
-            case BasicIo::end: newIdx = p_->size_ + offset; break;
-        }
-
-        if ( /*newIdx < 0 || */ newIdx > static_cast<uint64_t>(p_->size_) ) return 1;
-        p_->idx_ = static_cast<long>(newIdx);   //not very sure about this. need more test!!    - note by Shawn  fly2xj@gmail.com //TODO
-        p_->eof_ = false;
-        return 0;
-    }
-#else
-    int RemoteIo::seek(long offset, Position pos)
-    {
-        assert(p_->isMalloced_);
-        long newIdx = 0;
+        int64_t newIdx = 0;
 
         switch (pos) {
             case BasicIo::cur: newIdx = p_->idx_ + offset; break;
@@ -1950,13 +1884,12 @@ namespace Exiv2 {
 
         // #1198.  Don't return 1 when asked to seek past EOF.  Stay calm and set eof_
         // if (newIdx < 0 || newIdx > (long) p_->size_) return 1;
-        p_->idx_ = newIdx;
+        p_->idx_ = static_cast<long>(newIdx);
         p_->eof_ = newIdx > static_cast<long>(p_->size_);
         if (p_->idx_ > static_cast<long>(p_->size_))
             p_->idx_ = static_cast<long>(p_->size_);
         return 0;
     }
-#endif
 
     byte* RemoteIo::mmap(bool /*isWriteable*/)
     {

unitTests/test_FileIo.cpp

@@ -26,6 +26,7 @@ namespace
 {
     const std::string testData(TESTDATA_PATH);
     const std::string imagePath(testData + "/DSC_3079.jpg");
+    const std::string nonExistingImagePath(testData + "/nonExisting.jpg");
 }  // namespace
 
 TEST(AFileIO, canBeInstantiatedWithFilePath)
@@ -43,10 +44,27 @@ TEST(AFileIO, isOpenDoItsJob)
 {
     FileIo file(imagePath);
     ASSERT_FALSE(file.isopen());
-    file.open();
+    ASSERT_EQ(0, file.open());
     ASSERT_TRUE(file.isopen());
 }
 
+TEST(AFileIO, failsToOpenANonExistingFile)
+{
+    FileIo file(nonExistingImagePath);
+    ASSERT_FALSE(file.isopen());
+    ASSERT_EQ(1, file.open());
+    ASSERT_FALSE(file.isopen());
+}
+
+TEST(AFileIO, canChangeItsPathWithSetPath)
+{
+    FileIo file(nonExistingImagePath);
+    ASSERT_EQ(nonExistingImagePath, file.path());
+    file.setPath(imagePath);
+    ASSERT_EQ(imagePath, file.path());
+}
+
+
 TEST(AFileIO, returnsFileSizeIfItsOpened)
 {
     FileIo file(imagePath);