diff --git a/.gitignore b/.gitignore
index d3090138aa..702c4faec2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -23,4 +23,5 @@ src/doxygen.hpp
test/tmp/*
doc/html
contrib/vms/.vagrant
-/.vscode
\ No newline at end of file
+/.vscode
+.vs/
diff --git a/cmake/gcovr.cmake b/cmake/gcovr.cmake
index c787b88466..ad069563e7 100644
--- a/cmake/gcovr.cmake
+++ b/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)
diff --git a/include/exiv2/basicio.hpp b/include/exiv2/basicio.hpp
index b654891d20..1f6d4f0dc2 100644
--- a/include/exiv2/basicio.hpp
+++ b/include/exiv2/basicio.hpp
@@ -183,11 +183,8 @@ namespace Exiv2 {
@return 0 if successful;
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;
- 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;
- 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;
- 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
diff --git a/src/basicio.cpp b/src/basicio.cpp
index f7e699c6a8..7e32370515 100644
--- a/src/basicio.cpp
+++ b/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(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(p_->size_) ) return 1;
- p_->idx_ = static_cast(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(newIdx);
p_->eof_ = newIdx > static_cast(p_->size_);
if (p_->idx_ > static_cast(p_->size_))
p_->idx_ = static_cast(p_->size_);
return 0;
}
-#endif
byte* RemoteIo::mmap(bool /*isWriteable*/)
{
diff --git a/unitTests/test_FileIo.cpp b/unitTests/test_FileIo.cpp
index 67a5a4d89e..b5c1e7af4f 100644
--- a/unitTests/test_FileIo.cpp
+++ b/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);
diff --git a/unitTests/test_basicio.cpp b/unitTests/test_basicio.cpp
index 9b265c7441..d7b32185a2 100644
--- a/unitTests/test_basicio.cpp
+++ b/unitTests/test_basicio.cpp
@@ -21,80 +21,148 @@
#include
#include
+#include
+
using namespace Exiv2;
-TEST(MemIo, seek_out_of_bounds_00)
+TEST(MemIo, isNotAtEofInitially)
{
- byte buf[1024];
- memset(buf, 0, sizeof(buf));
+ std::array buf;
+ buf.fill(0);
- MemIo io(buf, sizeof(buf));
+ MemIo io(buf.data(), static_cast(buf.size()));
ASSERT_FALSE(io.eof());
+}
- // Regression test for bug reported in https://github.com/Exiv2/exiv2/pull/945
- // The problem is that MemIo::seek() does not check that the new offset is
- // in bounds.
- byte tmp[16];
- ASSERT_EQ(io.seek(0x10000000, BasicIo::beg), 1);
- ASSERT_TRUE(io.eof());
+TEST(MemIo, seekBeyondBufferSizeReturns1AndSetsEofToTrue)
+{
+ std::array buf;
+ buf.fill(0);
- // The seek was invalid, so the offset didn't change and this read still works.
- const long sizeTmp = static_cast(sizeof(sizeTmp));
- ASSERT_EQ(io.read(tmp, sizeTmp), sizeTmp);
+ MemIo io(buf.data(), static_cast(buf.size()));
+ ASSERT_EQ(1, io.seek(65, BasicIo::beg));
+ ASSERT_TRUE(io.eof());
}
-TEST(MemIo, seek_out_of_bounds_01)
+TEST(MemIo, seekBefore0Returns1ButItDoesNotSetEofToTrue)
{
- byte buf[1024];
- memset(buf, 0, sizeof(buf));
+ std::array buf;
+ buf.fill(0);
- MemIo io(buf, sizeof(buf));
+ MemIo io(buf.data(), static_cast(buf.size()));
+ ASSERT_EQ(1, io.seek(-1, BasicIo::beg));
ASSERT_FALSE(io.eof());
+}
+
+TEST(MemIo, seekBeyondBoundsDoesNotMoveThePosition)
+{
+ std::array buf;
+ buf.fill(0);
- byte tmp[16];
+ MemIo io(buf.data(), static_cast(buf.size()));
+ ASSERT_EQ(0, io.tell());
+ ASSERT_EQ(1, io.seek(65, BasicIo::beg));
+ ASSERT_EQ(0, io.tell());
+}
- // Seek to the end of the file.
- ASSERT_EQ(io.seek(0, BasicIo::end), 0);
- ASSERT_EQ(io.read(tmp, sizeof(tmp)), 0);
+TEST(MemIo, seekInsideBoundsMoveThePosition)
+{
+ std::array buf;
+ buf.fill(0);
- // Try to seek past the end of the file.
- ASSERT_EQ(io.seek(0x10000000, BasicIo::end), 1);
- ASSERT_TRUE(io.eof());
- ASSERT_EQ(io.read(tmp, sizeof(tmp)), 0);
+ MemIo io(buf.data(), static_cast(buf.size()));
+ ASSERT_EQ(0, io.tell());
+ ASSERT_EQ(0, io.seek(32, BasicIo::beg));
+ ASSERT_EQ(32, io.tell());
+}
+
+TEST(MemIo, seekInsideBoundsUsingBeg_resetsThePosition)
+{
+ std::array buf;
+ buf.fill(0);
+
+ MemIo io(buf.data(), static_cast(buf.size()));
+ std::vector positions {0, 8, 16, 32, 64};
+ for(auto pos: positions) {
+ ASSERT_EQ(0, io.seek(pos, BasicIo::beg));
+ ASSERT_EQ(pos, io.tell());
+ }
}
-TEST(MemIo, seek_out_of_bounds_02)
+TEST(MemIo, seekInsideBoundsUsingCur_shiftThePosition)
{
- byte buf[1024];
- memset(buf, 0, sizeof(buf));
+ std::array buf;
+ buf.fill(0);
+
+ MemIo io(buf.data(), static_cast(buf.size()));
+ std::vector shifts {4, 4, 8, 16, 32};
+ std::vector positions {4, 8, 16, 32, 64};
+ for (size_t i = 0; i < shifts.size(); ++i) {
+ ASSERT_EQ(0, io.seek(shifts[i], BasicIo::cur));
+ ASSERT_EQ(positions[i], io.tell());
+ }
+}
- MemIo io(buf, sizeof(buf));
+TEST(MemIo, seekToEndPosition_doesNotTriggerEof)
+{
+ std::array buf;
+ buf.fill(0);
+
+ MemIo io(buf.data(), static_cast(buf.size()));
+ ASSERT_EQ(0, io.tell());
+ ASSERT_EQ(0, io.seek(0, BasicIo::end));
+ ASSERT_EQ(64, io.tell());
ASSERT_FALSE(io.eof());
+}
+
+TEST(MemIo, seekToEndPositionAndReadTriggersEof)
+{
+ std::array buf;
+ buf.fill(0);
- byte tmp[16];
+ MemIo io(buf.data(), static_cast(buf.size()));
+ ASSERT_EQ(0, io.seek(0, BasicIo::end));
+ ASSERT_EQ(64, io.tell());
- // Try to seek past the end of the file.
- ASSERT_EQ(io.seek(0x10000000, BasicIo::cur), 1);
+ std::array buf2;
+ buf2.fill(0);
+ ASSERT_EQ(0, io.read(buf2.data(), 1)); // Note that we cannot even read 1 byte being at the end
ASSERT_TRUE(io.eof());
- // The seek was invalid, so the offset didn't change and this read still works.
- const long sizeTmp = static_cast(sizeof(sizeTmp));
- ASSERT_EQ(io.read(tmp, sizeTmp), sizeTmp);
}
-TEST(MemIo, seek_out_of_bounds_03)
+TEST(MemIo, readEmptyIoReturns0)
{
- byte buf[1024];
- memset(buf, 0, sizeof(buf));
+ std::array buf;
+ MemIo io;
+ ASSERT_EQ(0, io.read(buf.data(), static_cast(buf.size())));
+}
- MemIo io(buf, sizeof(buf));
- ASSERT_FALSE(io.eof());
+TEST(MemIo, readLessBytesThanAvailableReturnsRequestedBytes)
+{
+ std::array buf1, buf2;
+ buf1.fill(1);
+ buf2.fill(0);
- byte tmp[16];
+ MemIo io(buf1.data(), static_cast(buf1.size()));
+ ASSERT_EQ(5, io.read(buf2.data(), 5));
+}
- // Try to seek past the beginning of the file.
- ASSERT_EQ(io.seek(-0x10000000, BasicIo::cur), 1);
- ASSERT_FALSE(io.eof());
- // The seek was invalid, so the offset didn't change and this read still works.
- const long sizeTmp = static_cast(sizeof(sizeTmp));
- ASSERT_EQ(io.read(tmp, sizeTmp), sizeTmp);
+TEST(MemIo, readSameBytesThanAvailableReturnsRequetedBytes)
+{
+ std::array buf1, buf2;
+ buf1.fill(1);
+ buf2.fill(0);
+
+ MemIo io(buf1.data(), static_cast(buf1.size()));
+ ASSERT_EQ(10, io.read(buf2.data(), 10));
+}
+
+TEST(MemIo, readMoreBytesThanAvailableReturnsAvailableBytes)
+{
+ std::array buf1, buf2;
+ buf1.fill(1);
+ buf2.fill(0);
+
+ MemIo io(buf1.data(), static_cast(buf1.size()));
+ ASSERT_EQ(10, io.read(buf2.data(), 15));
}