third_party/exiv2
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Revisiting cmake code for generating coverage reports (#2047)

Browse Source 
* cmake: better usage of gcovr for coverage reports

* Add test for FileIo::setPath

* Remove useless seek() overload

* Add missing override specifiers

* ignore .vs folder

* Small refactors in BasicIo implementations

* Remove duplicated doxygen doc

* Refactor & add tests for MemIO

* Fix compilation warnings on windows
This commit is contained in:
Luis Díaz Más 2022-01-06 12:52:01 +01:00 committed by GitHub
parent a094dde1de
commit 813566526c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 169 additions and 183 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.gitignore vendored
View File

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

13
cmake/gcovr.cmake
View File

@ -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)

57
include/exiv2/basicio.hpp
View File

@ -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

85
src/basicio.cpp
View File

@ -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*/)
{

20
unitTests/test_FileIo.cpp
View File

@ -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);

174
unitTests/test_basicio.cpp
View File

@ -21,80 +21,148 @@
#include <exiv2/basicio.hpp>
#include <gtest/gtest.h>
#include <array>
using namespace Exiv2;
TEST(MemIo, seek_out_of_bounds_00)
TEST(MemIo, isNotAtEofInitially)
{
byte buf[1024];
memset(buf, 0, sizeof(buf));
std::array<byte, 64> buf;
buf.fill(0);
MemIo io(buf, sizeof(buf));
MemIo io(buf.data(), static_cast<long>(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);
TEST(MemIo, seekBeyondBufferSizeReturns1AndSetsEofToTrue)
{
std::array<byte, 64> buf;
buf.fill(0);
MemIo io(buf.data(), static_cast<long>(buf.size()));
ASSERT_EQ(1, io.seek(65, BasicIo::beg));
ASSERT_TRUE(io.eof());
// The seek was invalid, so the offset didn't change and this read still works.
const long sizeTmp = static_cast<long>(sizeof(sizeTmp));
ASSERT_EQ(io.read(tmp, sizeTmp), sizeTmp);
}
TEST(MemIo, seek_out_of_bounds_01)
TEST(MemIo, seekBefore0Returns1ButItDoesNotSetEofToTrue)
{
byte buf[1024];
memset(buf, 0, sizeof(buf));
std::array<byte, 64> buf;
buf.fill(0);
MemIo io(buf, sizeof(buf));
MemIo io(buf.data(), static_cast<long>(buf.size()));
ASSERT_EQ(1, io.seek(-1, BasicIo::beg));
ASSERT_FALSE(io.eof());
}
byte tmp[16];
TEST(MemIo, seekBeyondBoundsDoesNotMoveThePosition)
{
std::array<byte, 64> buf;
buf.fill(0);
// Seek to the end of the file.
ASSERT_EQ(io.seek(0, BasicIo::end), 0);
ASSERT_EQ(io.read(tmp, sizeof(tmp)), 0);
MemIo io(buf.data(), static_cast<long>(buf.size()));
ASSERT_EQ(0, io.tell());
ASSERT_EQ(1, io.seek(65, BasicIo::beg));
ASSERT_EQ(0, io.tell());
}
// Try to seek past the end of the file.
ASSERT_EQ(io.seek(0x10000000, BasicIo::end), 1);
TEST(MemIo, seekInsideBoundsMoveThePosition)
{
std::array<byte, 64> buf;
buf.fill(0);
MemIo io(buf.data(), static_cast<long>(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<byte, 64> buf;
buf.fill(0);
MemIo io(buf.data(), static_cast<long>(buf.size()));
std::vector<std::int64_t> 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, seekInsideBoundsUsingCur_shiftThePosition)
{
std::array<byte, 64> buf;
buf.fill(0);
MemIo io(buf.data(), static_cast<long>(buf.size()));
std::vector<std::int64_t> shifts {4, 4, 8, 16, 32};
std::vector<std::int64_t> 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());
}
}
TEST(MemIo, seekToEndPosition_doesNotTriggerEof)
{
std::array<byte, 64> buf;
buf.fill(0);
MemIo io(buf.data(), static_cast<long>(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<byte, 64> buf;
buf.fill(0);
MemIo io(buf.data(), static_cast<long>(buf.size()));
ASSERT_EQ(0, io.seek(0, BasicIo::end));
ASSERT_EQ(64, io.tell());
std::array<byte, 64> 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());
ASSERT_EQ(io.read(tmp, sizeof(tmp)), 0);
}
TEST(MemIo, seek_out_of_bounds_02)
TEST(MemIo, readEmptyIoReturns0)
{
byte buf[1024];
memset(buf, 0, sizeof(buf));
MemIo io(buf, sizeof(buf));
ASSERT_FALSE(io.eof());
byte tmp[16];
// Try to seek past the end of the file.
ASSERT_EQ(io.seek(0x10000000, BasicIo::cur), 1);
ASSERT_TRUE(io.eof());
// The seek was invalid, so the offset didn't change and this read still works.
const long sizeTmp = static_cast<long>(sizeof(sizeTmp));
ASSERT_EQ(io.read(tmp, sizeTmp), sizeTmp);
std::array<byte, 10> buf;
MemIo io;
ASSERT_EQ(0, io.read(buf.data(), static_cast<long>(buf.size())));
}
TEST(MemIo, seek_out_of_bounds_03)
TEST(MemIo, readLessBytesThanAvailableReturnsRequestedBytes)
{
byte buf[1024];
memset(buf, 0, sizeof(buf));
std::array<byte, 10> buf1, buf2;
buf1.fill(1);
buf2.fill(0);
MemIo io(buf, sizeof(buf));
ASSERT_FALSE(io.eof());
byte tmp[16];
// 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<long>(sizeof(sizeTmp));
ASSERT_EQ(io.read(tmp, sizeTmp), sizeTmp);
MemIo io(buf1.data(), static_cast<long>(buf1.size()));
ASSERT_EQ(5, io.read(buf2.data(), 5));
}
TEST(MemIo, readSameBytesThanAvailableReturnsRequetedBytes)
{
std::array<byte, 10> buf1, buf2;
buf1.fill(1);
buf2.fill(0);
MemIo io(buf1.data(), static_cast<long>(buf1.size()));
ASSERT_EQ(10, io.read(buf2.data(), 10));
}
TEST(MemIo, readMoreBytesThanAvailableReturnsAvailableBytes)
{
std::array<byte, 10> buf1, buf2;
buf1.fill(1);
buf2.fill(0);
MemIo io(buf1.data(), static_cast<long>(buf1.size()));
ASSERT_EQ(10, io.read(buf2.data(), 15));
}