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.

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

Refactoring & cleanup #2109

Merged
merged 25 commits into from
Feb 21, 2022
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
3d370cc
Improvements in urlencode
piponazo Feb 16, 2022
f1ff3aa
Make urldecode in-place
piponazo Feb 16, 2022
6f762b4
Use std::filesystem for fileExist
piponazo Feb 16, 2022
0726104
Hide pathOfFileUrl in the only place where it is used
piponazo Feb 16, 2022
45300ad
BasicIo::path() returns const ref
piponazo Feb 16, 2022
8b2d173
ReplaceStringInPlace does it in-place now
piponazo Feb 16, 2022
9b3a643
Use rename from filesystem
piponazo Feb 16, 2022
56b5ab9
Use remove from filesystem
piponazo Feb 16, 2022
a6185d2
Image::setComment now takes string_view
piponazo Feb 16, 2022
798cf9b
Remove dead code (winNumberOfLinks)
piponazo Feb 17, 2022
c0b663b
Remove dead code (LSTAT)
piponazo Feb 17, 2022
1d243ed
Remove dead code: copyXattrFrom
piponazo Feb 17, 2022
ea201ce
Remove dead code
piponazo Feb 17, 2022
d11479e
Replace dynamic C array by std::vector
piponazo Feb 17, 2022
59f4d0d
cppcheck: reduce scope of variables
piponazo Feb 17, 2022
476a5e2
Replace raw loop for any_of
piponazo Feb 17, 2022
b543f3e
Use filesystem in getExiv2ConfigPath
piponazo Feb 18, 2022
7e5ba7c
Remove many redundant or not needed header inclusions
piponazo Feb 18, 2022
7caf409
Use fs::file_size instead of stat
piponazo Feb 18, 2022
f060b58
Clean config.h from old stuff
piponazo Feb 18, 2022
76f01fd
Clean more header inclusions
piponazo Feb 18, 2022
9fb43f2
Use standard [[maybe_unused]]
piponazo Feb 18, 2022
f774a3b
Fix build on linux
piponazo Feb 18, 2022
8b3da36
Improvements from code review
piponazo Feb 19, 2022
21eb0ce
Fix build when EXIV2_BUILD_MESSAGES is ON
piponazo Feb 19, 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
57 changes: 25 additions & 32 deletions app/actions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ namespace {
public:
//! C'tor
Timestamp() = default;
//! Read the timestamp of a file
int read(const std::string& path);
//! Read the timestamp from a broken-down time in buffer \em tm.
int read(struct tm* tm);
Expand Down Expand Up @@ -268,7 +267,7 @@ namespace Action {

int Print::printSummary()
{
if (!Exiv2::fileExists(path_, true)) {
if (!Exiv2::fileExists(path_)) {
std::cerr << path_ << ": " << _("Failed to open the file\n");
return -1;
}
Expand Down Expand Up @@ -402,7 +401,7 @@ namespace Action {

int Print::printList()
{
if (!Exiv2::fileExists(path_, true)) {
if (!Exiv2::fileExists(path_)) {
std::cerr << path_ << ": " << _("Failed to open the file\n");
return -1;
}
Expand Down Expand Up @@ -607,7 +606,7 @@ namespace Action {

int Print::printComment()
{
if (!Exiv2::fileExists(path_, true)) {
if (!Exiv2::fileExists(path_)) {
std::cerr << path_ << ": " << _("Failed to open the file\n");
return -1;
}
Expand All @@ -624,7 +623,7 @@ namespace Action {

int Print::printPreviewList()
{
if (!Exiv2::fileExists(path_, true)) {
if (!Exiv2::fileExists(path_)) {
std::cerr << path_ << ": " << _("Failed to open the file\n");
return -1;
}
Expand Down Expand Up @@ -658,7 +657,7 @@ namespace Action {
int Rename::run(const std::string& path)
{
try {
if (!Exiv2::fileExists(path, true)) {
if (!Exiv2::fileExists(path)) {
std::cerr << path << ": " << _("Failed to open the file\n");
return -1;
}
Expand Down Expand Up @@ -740,7 +739,7 @@ namespace Action {
try {
path_ = path;

if (!Exiv2::fileExists(path_, true)) {
if (!Exiv2::fileExists(path_)) {
std::cerr << path_ << ": " << _("Failed to open the file\n");
return -1;
}
Expand Down Expand Up @@ -900,7 +899,7 @@ namespace Action {

int Extract::writeThumbnail() const
{
if (!Exiv2::fileExists(path_, true)) {
if (!Exiv2::fileExists(path_)) {
std::cerr << path_ << ": " << _("Failed to open the file\n");
return -1;
}
Expand Down Expand Up @@ -947,7 +946,7 @@ namespace Action {

int Extract::writePreviews() const
{
if (!Exiv2::fileExists(path_, true)) {
if (!Exiv2::fileExists(path_)) {
std::cerr << path_ << ": " << _("Failed to open the file\n");
return -1;
}
Expand Down Expand Up @@ -983,7 +982,7 @@ namespace Action {
int Extract::writeIccProfile(const std::string& target) const
{
int rc = 0;
if (!Exiv2::fileExists(path_, true)) {
if (!Exiv2::fileExists(path_)) {
std::cerr << path_ << ": " << _("Failed to open the file\n");
rc = -1;
}
Expand Down Expand Up @@ -1049,7 +1048,7 @@ namespace Action {
// -i{tgt}- reading from stdin?
bool bStdin = (Params::instance().target_ & Params::ctStdInOut) != 0;

if (!Exiv2::fileExists(path, true)) {
if (!Exiv2::fileExists(path)) {
std::cerr << path
<< ": " << _("Failed to open the file\n");
return -1;
Expand Down Expand Up @@ -1106,12 +1105,12 @@ namespace Action {
Params::instance().getStdin(xmpBlob);
rc = insertXmpPacket(path,xmpBlob,true);
} else {
if (!Exiv2::fileExists(xmpPath, true)) {
if (!Exiv2::fileExists(xmpPath)) {
std::cerr << xmpPath
<< ": " << _("Failed to open the file\n");
rc = -1;
}
if (rc == 0 && !Exiv2::fileExists(path, true)) {
if (rc == 0 && !Exiv2::fileExists(path)) {
std::cerr << path
<< ": " << _("Failed to open the file\n");
rc = -1;
Expand Down Expand Up @@ -1152,7 +1151,7 @@ namespace Action {
Params::instance().getStdin(iccProfile);
rc = insertIccProfile(path,std::move(iccProfile));
} else {
if (!Exiv2::fileExists(iccProfilePath, true)) {
if (!Exiv2::fileExists(iccProfilePath)) {
std::cerr << iccProfilePath
<< ": " << _("Failed to open the file\n");
rc = -1;
Expand All @@ -1168,7 +1167,7 @@ namespace Action {
{
int rc = 0;
// test path exists
if (!Exiv2::fileExists(path, true)) {
if (!Exiv2::fileExists(path)) {
std::cerr << path << ": " << _("Failed to open the file\n");
rc=-1;
}
Expand All @@ -1192,12 +1191,12 @@ namespace Action {
int Insert::insertThumbnail(const std::string& path)
{
std::string thumbPath = newFilePath(path, "-thumb.jpg");
if (!Exiv2::fileExists(thumbPath, true)) {
if (!Exiv2::fileExists(thumbPath)) {
std::cerr << thumbPath
<< ": " << _("Failed to open the file\n");
return -1;
}
if (!Exiv2::fileExists(path, true)) {
if (!Exiv2::fileExists(path)) {
std::cerr << path
<< ": " << _("Failed to open the file\n");
return -1;
Expand All @@ -1220,7 +1219,7 @@ namespace Action {
int Modify::run(const std::string& path)
{
try {
if (!Exiv2::fileExists(path, true)) {
if (!Exiv2::fileExists(path)) {
std::cerr << path << ": " << _("Failed to open the file\n");
return -1;
}
Expand Down Expand Up @@ -1452,7 +1451,7 @@ namespace Action {
monthAdjustment_ = Params::instance().yodAdjust_[Params::yodMonth].adjustment_;
dayAdjustment_ = Params::instance().yodAdjust_[Params::yodDay].adjustment_;

if (!Exiv2::fileExists(path, true)) {
if (!Exiv2::fileExists(path)) {
std::cerr << path << ": " << _("Failed to open the file\n");
return -1;
}
Expand Down Expand Up @@ -1579,7 +1578,7 @@ namespace Action {
int FixIso::run(const std::string& path)
{
try {
if (!Exiv2::fileExists(path, true)) {
if (!Exiv2::fileExists(path)) {
std::cerr << path << ": " <<_("Failed to open the file\n");
return -1;
}
Expand Down Expand Up @@ -1632,7 +1631,7 @@ namespace Action {
int FixCom::run(const std::string& path)
{
try {
if (!Exiv2::fileExists(path, true)) {
if (!Exiv2::fileExists(path)) {
std::cerr << path << ": " <<_("Failed to open the file\n");
return -1;
}
Expand Down Expand Up @@ -1821,7 +1820,7 @@ namespace {

// read the source metadata
int rc = -1 ;
if (!Exiv2::fileExists(source, true)) {
if (!Exiv2::fileExists(source)) {
std::cerr << source << ": " << _("Failed to open the file\n");
return rc;
}
Expand Down Expand Up @@ -1946,7 +1945,8 @@ namespace {
}

// delete temporary target
if ( bStdout ) std::remove(target.c_str());
if ( bStdout )
fs::remove(target.c_str());

return rc;
} // metacopy
Expand Down Expand Up @@ -2046,14 +2046,7 @@ namespace {
std::cout << std::endl;
}

// Workaround for MinGW rename which does not overwrite existing files
remove(newPath.c_str());
if (std::rename(path.c_str(), newPath.c_str()) == -1) {
std::cerr << Params::instance().progname() << ": " << _("Failed to rename") << " " << path << " " << _("to")
<< " " << newPath << ": " << Exiv2::strError() << "\n";
return 1;
}

fs::rename(path, newPath);
return 0;
}

Expand Down Expand Up @@ -2096,7 +2089,7 @@ namespace {

int printStructure(std::ostream& out, Exiv2::PrintStructureOption option, const std::string &path)
{
if (!Exiv2::fileExists(path, true)) {
if (!Exiv2::fileExists(path)) {
std::cerr << path << ": "
<< _("Failed to open the file\n");
return -1;
Expand Down
14 changes: 4 additions & 10 deletions include/exiv2/basicio.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ namespace Exiv2 {
comprehensive error messages where only a BasicIo instance is
available.
*/
virtual std::string path() const =0;
virtual const std::string& path() const noexcept =0;

/*!
@brief Mark all the bNone blocks to bKnow. This avoids allocating memory
Expand Down Expand Up @@ -472,7 +472,7 @@ namespace Exiv2 {
//! Returns true if the file position has reached the end, otherwise false.
bool eof() const override;
//! Returns the path of the file
std::string path() const override;
const std::string& path() const noexcept override;

/*!
@brief Mark all the bNone blocks to bKnow. This avoids allocating memory
Expand Down Expand Up @@ -654,7 +654,7 @@ namespace Exiv2 {
//!Returns true if the IO position has reached the end, otherwise false.
bool eof() const override;
//! Returns a dummy path, indicating that memory access is used
std::string path() const override;
const std::string& path() const noexcept override;

/*!
@brief Mark all the bNone blocks to bKnow. This avoids allocating memory
Expand Down Expand Up @@ -898,7 +898,7 @@ namespace Exiv2 {
//!Returns true if the IO position has reached the end, otherwise false.
bool eof() const override;
//!Returns the URL of the file.
std::string path() const override;
const std::string& path() const noexcept override;

/*!
@brief Mark all the bNone blocks to bKnow. This avoids allocating memory
Expand Down Expand Up @@ -1008,12 +1008,6 @@ namespace Exiv2 {
@throw Error In case of failure.
*/
EXIV2API long writeFile(const DataBuf& buf, const std::string& path);
/*!
@brief replace each substring of the subject that matches the given search string with the given replacement.
@return the subject after replacing.
*/
EXIV2API std::string ReplaceStringInPlace(std::string subject, const std::string& search,
const std::string& replace);
#ifdef EXV_USE_CURL
/*!
@brief The callback function is called by libcurl to write the data
Expand Down
2 changes: 1 addition & 1 deletion include/exiv2/bmffimage.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ namespace Exiv2
//@{
void readMetadata() override /* override */;
void writeMetadata() override /* override */;
void setComment(const std::string& comment) override /* override */;
void setComment(const std::string_view comment) override /* override */;
piponazo marked this conversation as resolved.
Show resolved Hide resolved
void printStructure(std::ostream& out, Exiv2::PrintStructureOption option, int depth) override;
//@}

Expand Down
2 changes: 1 addition & 1 deletion include/exiv2/bmpimage.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ namespace Exiv2 {
void setIptcData(const IptcData& iptcData) override;

/// @throws Error(kerInvalidSettingForImage)
void setComment(const std::string& comment) override;
void setComment(const std::string_view comment) override;
//@}

//! @name Accessors
Expand Down
2 changes: 1 addition & 1 deletion include/exiv2/cr2image.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ namespace Exiv2 {
@brief Not supported. CR2 format does not contain a comment.
Calling this function will throw an Error(kerInvalidSettingForImage).
*/
void setComment(const std::string& comment) override;
void setComment(const std::string_view comment) override;
//@}

//! @name Accessors
Expand Down
2 changes: 1 addition & 1 deletion include/exiv2/epsimage.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ namespace Exiv2
@brief Not supported.
Calling this function will throw an instance of Error(kerInvalidSettingForImage).
*/
void setComment(const std::string& comment) override;
void setComment(const std::string_view comment) override;
//@}

//! @name Accessors
Expand Down
23 changes: 2 additions & 21 deletions include/exiv2/futils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,7 @@ namespace Exiv2
@note Source: http://www.geekhideout.com/urlcode.shtml
@todo This function can probably be hidden into the implementation details
*/
EXIV2API std::string urlencode(const char* str);

/*!
@brief Decode the input url.
@param str The url needs decoding.
@return the url-decoded version of str.

@note Be sure to 'free' the returned string after use with 'delete []'.
Source: http://www.geekhideout.com/urlcode.shtml
@todo This function can probably be hidden into the implementation details
*/
EXIV2API char* urldecode(const char* str);
EXIV2API std::string urlencode(const std::string_view& str);
piponazo marked this conversation as resolved.
Show resolved Hide resolved

/*!
@brief Like urlencode(char* str) but accept the input url in the std::string and modify it.
Expand Down Expand Up @@ -125,15 +114,7 @@ namespace Exiv2
and its type, see stat(2). <b>errno</b> is left unchanged
in case of an error.
*/
EXIV2API bool fileExists(const std::string& path, bool ct = false);

/*!
@brief Get the path of file URL.

@param url The file URL in the format file:///path or file://host/path
@return the path of file URL.
*/
EXIV2API std::string pathOfFileUrl(const std::string& url);
EXIV2API bool fileExists(const std::string& path);

/*!
@brief Return a system error message and the error code (errno).
Expand Down
2 changes: 1 addition & 1 deletion include/exiv2/gifimage.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ namespace Exiv2 {
@brief Not supported. Calling this function will throw an instance
of Error(kerInvalidSettingForImage).
*/
void setComment(const std::string& comment) override;
void setComment(const std::string_view comment) override;
//@}

//! @name Accessors
Expand Down
10 changes: 4 additions & 6 deletions include/exiv2/image.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -199,12 +199,10 @@ namespace Exiv2 {
writeXmpFromPacket(true) or setXmpPacket().
*/
virtual void clearXmpData();
/*!
@brief Set the image comment. The new comment is not written
to the image until the writeMetadata() method is called.
@param comment String containing comment.
*/
virtual void setComment(const std::string& comment);

/// @brief Set the image comment. The comment is written to the image when writeMetadata() is called.
virtual void setComment(const std::string_view comment);

/*!
@brief Erase any buffered comment. Comment is not removed
from the actual image until the writeMetadata() method is called.
Expand Down
2 changes: 1 addition & 1 deletion include/exiv2/jp2image.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ namespace Exiv2
@brief Todo: Not supported yet(?). Calling this function will throw
an instance of Error(kerInvalidSettingForImage).
*/
void setComment(const std::string& comment) override;
void setComment(const std::string_view comment) override;
//@}

//! @name Accessors
Expand Down
2 changes: 1 addition & 1 deletion include/exiv2/mrwimage.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ namespace Exiv2 {
@brief Not supported. MRW format does not contain a comment.
Calling this function will throw an Error(kerInvalidSettingForImage).
*/
void setComment(const std::string& comment) override;
void setComment(const std::string_view comment) override;
//@}

//! @name Accessors
Expand Down
Loading