Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

misc cleanups #2309

Merged
merged 8 commits into from
Aug 5, 2022
Merged

misc cleanups #2309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
18cf147
simplify switch statement with returns
neheb Aug 3, 2022
a70e0f6
enum class conversion
neheb Aug 3, 2022
4bcf526
remove unused using
neheb Aug 3, 2022
75071c9
remove explicit constructor
neheb Aug 3, 2022
dccfd96
default init static variable
neheb Aug 4, 2022
62d07ec
const conversions
neheb Aug 4, 2022
8909dd0
misc sonarlint cleanups
neheb Aug 5, 2022
8a73274
remove c_str for fs stuff
neheb Aug 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 11 additions & 13 deletions src/basicio.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ void FileIo::transfer(BasicIo& src) {
// Check if the file can be written to, if it already exists
if (open("a+b") != 0) {
// Remove the (temporary) file
fs::remove(fileIo->path().c_str());
fs::remove(fileIo->path());
throw Error(ErrorCode::kerFileOpenFailed, path(), "a+b", strError());
}
close();
Expand Down Expand Up @@ -366,8 +366,8 @@ void FileIo::transfer(BasicIo& src) {
pfcn_ReplaceFileA(pf, fileIo->path().c_str(), nullptr, REPLACEFILE_IGNORE_MERGE_ERRORS, nullptr, nullptr);
if (ret == 0) {
if (GetLastError() == ERROR_FILE_NOT_FOUND) {
fs::rename(fileIo->path().c_str(), pf);
fs::remove(fileIo->path().c_str());
fs::rename(fileIo->path(), pf);
fs::remove(fileIo->path());
} else {
throw Error(ErrorCode::kerFileRenameFailed, fileIo->path(), pf, strError());
}
Expand All @@ -376,16 +376,16 @@ void FileIo::transfer(BasicIo& src) {
if (fileExists(pf) && ::remove(pf) != 0) {
throw Error(ErrorCode::kerCallFailed, pf, strError(), "fs::remove");
}
fs::rename(fileIo->path().c_str(), pf);
fs::remove(fileIo->path().c_str());
fs::rename(fileIo->path(), pf);
fs::remove(fileIo->path());
}
}
#else
if (fileExists(pf) && fs::remove(pf) != 0) {
throw Error(ErrorCode::kerCallFailed, pf, strError(), "fs::remove");
}
fs::rename(fileIo->path().c_str(), pf);
fs::remove(fileIo->path().c_str());
fs::rename(fileIo->path(), pf);
fs::remove(fileIo->path());
#endif
// Check permissions of new file
struct stat buf2;
Expand All @@ -395,13 +395,11 @@ void FileIo::transfer(BasicIo& src) {
EXV_WARNING << Error(ErrorCode::kerCallFailed, pf, strError(), "::stat") << "\n";
#endif
}
if (statOk && origStMode != buf2.st_mode) {
// Set original file permissions
if (::chmod(pf, origStMode) == -1) {
// Set original file permissions
if (statOk && origStMode != buf2.st_mode && ::chmod(pf, origStMode) == -1) {

Check failure

Code scanning / CodeQL

Time-of-check time-of-use filesystem race condition

The [filename](1) being operated upon was previously [checked](2), but the underlying file may have been changed since then.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hahaha no.

#ifndef SUPPRESS_WARNINGS
EXV_WARNING << Error(ErrorCode::kerCallFailed, pf, strError(), "::chmod") << "\n";
EXV_WARNING << Error(ErrorCode::kerCallFailed, pf, strError(), "::chmod") << "\n";
#endif
}
}
}
} // if (fileIo)
Expand Down Expand Up @@ -1316,7 +1314,7 @@ byte* RemoteIo::mmap(bool /*isWriteable*/) {
size_t blocks = (p_->size_ + blockSize - 1) / blockSize;
bigBlock_ = new byte[blocks * blockSize];
for (size_t block = 0; block < blocks; block++) {
void* p = p_->blocksMap_[block].getData();
auto p = p_->blocksMap_[block].getData();
if (p) {
size_t nRead = block == (blocks - 1) ? p_->size_ - nRealData : blockSize;
memcpy(bigBlock_ + (block * blockSize), p, nRead);
Expand Down
39 changes: 21 additions & 18 deletions src/bmffimage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,13 @@ std::string BmffImage::uuidName(Exiv2::DataBuf& uuid) {
const char* uuidCano = "\x85\xC0\xB6\x87\x82\xF\x11\xE0\x81\x11\xF4\xCE\x46\x2B\x6A\x48";
const char* uuidXmp = "\xBE\x7A\xCF\xCB\x97\xA9\x42\xE8\x9C\x71\x99\x94\x91\xE3\xAF\xAC";
const char* uuidCanp = "\xEA\xF4\x2B\x5E\x1C\x98\x4B\x88\xB9\xFB\xB7\xDC\x40\x6E\x4D\x16";
const char* result = uuid.cmpBytes(0, uuidCano, 16) == 0 ? "cano"
: uuid.cmpBytes(0, uuidXmp, 16) == 0 ? "xmp"
: uuid.cmpBytes(0, uuidCanp, 16) == 0 ? "canp"
: "";
return result;
if (uuid.cmpBytes(0, uuidCano, 16) == 0)
return "cano";
if (uuid.cmpBytes(0, uuidXmp, 16) == 0)
return "xmp";
if (uuid.cmpBytes(0, uuidCanp, 16) == 0)
return "canp";
return "";
}

uint64_t BmffImage::boxHandler(std::ostream& out /* = std::cout*/, Exiv2::PrintStructureOption option /* = kpsNone */,
Expand Down Expand Up @@ -345,9 +347,13 @@ uint64_t BmffImage::boxHandler(std::ostream& out /* = std::cout*/, Exiv2::PrintS
enforce(data.size() - skip >= (version > 2u ? 4u : 2u), Exiv2::ErrorCode::kerCorruptedMetadata);
enforce(data.size() - skip >= step, Exiv2::ErrorCode::kerCorruptedMetadata);
uint32_t ID = version > 2 ? data.read_uint32(skip, endian_) : data.read_uint16(skip, endian_);
uint32_t offset = step == 14 || step == 16 ? data.read_uint32(skip + step - 8, endian_)
: step == 18 ? data.read_uint32(skip + 4, endian_)
: 0;
auto offset = [=] {
if (step == 14 || step == 16)
return data.read_uint32(skip + step - 8, endian_);
if (step == 18)
return data.read_uint32(skip + 4, endian_);
return 0u;
}();

uint32_t ldata = data.read_uint32(skip + step - 4, endian_);
if (bTrace) {
Expand Down Expand Up @@ -388,7 +394,7 @@ uint64_t BmffImage::boxHandler(std::ostream& out /* = std::cout*/, Exiv2::PrintS
uint8_t meth = data.read_uint8(skip + 0);
uint8_t prec = data.read_uint8(skip + 1);
uint8_t approx = data.read_uint8(skip + 2);
std::string colour_type = std::string(data.c_str(), 4);
auto colour_type = std::string(data.c_str(), 4);
skip += 4;
if (colour_type == "rICC" || colour_type == "prof") {
DataBuf profile(data.c_data(skip), data.size() - skip);
Expand Down Expand Up @@ -554,14 +560,11 @@ void BmffImage::parseCr3Preview(DataBuf& data, std::ostream& out, bool bTrace, u
nativePreview.height_ = data.read_uint16(height_offset, endian_);
nativePreview.size_ = data.read_uint32(size_offset, endian_);
nativePreview.filter_ = "";
switch (version) {
case 0:
nativePreview.mimeType_ = "image/jpeg";
break;
default:
nativePreview.mimeType_ = "application/octet-stream";
break;
}
nativePreview.mimeType_ = [version] {
if (version == 0)
return "image/jpeg";
return "application/octet-stream";
}();
nativePreviews_.push_back(nativePreview);

if (bTrace) {
Expand Down Expand Up @@ -639,7 +642,7 @@ void BmffImage::printStructure(std::ostream& out, Exiv2::PrintStructureOption op
io_->seek(address, BasicIo::beg);
address = boxHandler(out, option, file_end, depth);
}
}; break;
} break;
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/canonmn_int.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2582,7 +2582,7 @@ std::ostream& printCsLensFFFF(std::ostream& os, const Value& value, const ExifDa
return os << "Canon EF-S 24mm f/2.8 STM";
}
} catch (const std::exception&) {
};
}

return EXV_PRINT_TAG(canonCsLensType)(os, value, metadata);
}
Expand Down
41 changes: 16 additions & 25 deletions src/crwimage_int.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,6 @@ const CrwSubDir CrwMap::crwSubDir_[] = {
{0x2807, 0x300a}, {0x2804, 0x300a}, {0x300a, 0x0000}, {0x0000, 0xffff},
};

const char CiffHeader::signature_[] = "HEAPCCDR";

CiffDirectory::~CiffDirectory() {
for (auto&& component : components_) {
delete component;
Expand Down Expand Up @@ -185,7 +183,7 @@ void CiffComponent::doRead(const byte* pData, size_t size, uint32_t start, ByteO

DataLocId dl = dataLocation();

if (dl == valueData) {
if (dl == DataLocId::valueData) {
size_ = getULong(pData + start + 2, byteOrder);
offset_ = getULong(pData + start + 6, byteOrder);

Expand All @@ -204,7 +202,7 @@ void CiffComponent::doRead(const byte* pData, size_t size, uint32_t start, ByteO
enforce(size_ <= size - offset_, ErrorCode::kerOffsetOutOfRange);
}
}
if (dl == directoryData) {
if (dl == DataLocId::directoryData) {
size_ = 8;
offset_ = start + 2;
}
Expand Down Expand Up @@ -316,7 +314,7 @@ size_t CiffEntry::doWrite(Blob& blob, ByteOrder /*byteOrder*/, size_t offset) {
}

size_t CiffComponent::writeValueData(Blob& blob, size_t offset) {
if (dataLocation() == valueData) {
if (dataLocation() == DataLocId::valueData) {
#ifdef EXIV2_DEBUG_MESSAGES
std::cout << " Data for tag 0x" << std::hex << tagId() << ", " << std::dec << size_ << " Bytes\n";
#endif
Expand Down Expand Up @@ -382,7 +380,7 @@ void CiffComponent::writeDirEntry(Blob& blob, ByteOrder byteOrder) const {

DataLocId dl = dataLocation();

if (dl == valueData) {
if (dl == DataLocId::valueData) {
us2Data(buf, tag_, byteOrder);
append(blob, buf, 2);

Expand All @@ -393,7 +391,7 @@ void CiffComponent::writeDirEntry(Blob& blob, ByteOrder byteOrder) const {
append(blob, buf, 4);
}

if (dl == directoryData) {
if (dl == DataLocId::directoryData) {
// Only 8 bytes fit in the directory entry

us2Data(buf, tag_, byteOrder);
Expand Down Expand Up @@ -437,43 +435,36 @@ void CiffComponent::setValue(DataBuf&& buf) {
storage_ = std::move(buf);
pData_ = storage_.c_data();
size_ = storage_.size();
if (size_ > 8 && dataLocation() == directoryData) {
if (size_ > 8 && dataLocation() == DataLocId::directoryData) {
tag_ &= 0x3fff;
}
}

TypeId CiffComponent::typeId(uint16_t tag) {
TypeId ti = invalidTypeId;
switch (tag & 0x3800) {
case 0x0000:
ti = unsignedByte;
break;
return unsignedByte;
case 0x0800:
ti = asciiString;
break;
return asciiString;
case 0x1000:
ti = unsignedShort;
break;
return unsignedShort;
case 0x1800:
ti = unsignedLong;
break;
return unsignedLong;
case 0x2000:
ti = undefined;
break;
case 0x2800: // fallthrough
return undefined;
case 0x2800:
case 0x3000:
ti = directory;
break;
return directory;
}
return ti;
return invalidTypeId;
} // CiffComponent::typeId

DataLocId CiffComponent::dataLocation(uint16_t tag) {
switch (tag & 0xc000) {
case 0x0000:
return valueData;
return DataLocId::valueData;
case 0x4000:
return directoryData;
return DataLocId::directoryData;
default:
throw Error(ErrorCode::kerCorruptedMetadata);
}
Expand Down
4 changes: 2 additions & 2 deletions src/crwimage_int.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ using CrwEncodeFct = std::function<void(const Image&, const CrwMapping*, CiffHea
using CrwDirs = std::stack<CrwSubDir>;

//! Type to identify where the data is stored in a directory
enum DataLocId { valueData, directoryData, lastDataLocId };
enum class DataLocId { valueData, directoryData, lastDataLocId };

// *****************************************************************************
// class definitions
Expand Down Expand Up @@ -480,7 +480,7 @@ class CiffHeader {

private:
// DATA
static const char signature_[]; //!< Canon CRW signature "HEAPCCDR"
static constexpr auto signature_ = "HEAPCCDR"; //!< Canon CRW signature

std::unique_ptr<CiffDirectory> pRootDir_; //!< Pointer to the root directory
ByteOrder byteOrder_ = littleEndian; //!< Applicable byte order
Expand Down
Loading