From 58f7cd8644bab1fbd89811a4e3a03d4f6e21d05b Mon Sep 17 00:00:00 2001 From: Willy Scheibel Date: Thu, 6 Apr 2017 22:58:49 +0200 Subject: [PATCH] Review --- source/cppfs/CMakeLists.txt | 1 + .../cppfs/include/cppfs/AbstractFileSystem.h | 9 ++++ source/cppfs/include/cppfs/Change.h | 31 ++++++++++- source/cppfs/include/cppfs/Diff.h | 30 +++++++++++ source/cppfs/include/cppfs/FileHandle.h | 11 +++- source/cppfs/include/cppfs/FilePath.h | 21 +++++--- source/cppfs/include/cppfs/LoginCredentials.h | 15 +++++- source/cppfs/include/cppfs/Tree.h | 33 ++++++++++-- source/cppfs/include/cppfs/Url.h | 22 ++++---- .../include/cppfs/posix/LocalFileHandle.h | 11 ++++ .../include/cppfs/posix/LocalFileIterator.h | 11 ++++ .../include/cppfs/posix/LocalFileSystem.h | 1 + .../cppfs/include/cppfs/ssh/SshFileSystem.h | 27 ++++++++++ .../include/cppfs/ssh/SshInputStreamBuffer.h | 5 +- .../include/cppfs/ssh/SshOutputStreamBuffer.h | 11 ++-- source/cppfs/include/cppfs/system.h | 10 +++- source/cppfs/include/cppfs/units.h | 35 ++++++++++++ source/cppfs/source/Change.cpp | 22 +++++++- source/cppfs/source/Diff.cpp | 21 ++++++-- source/cppfs/source/FileHandle.cpp | 53 +++++++++++++------ source/cppfs/source/FilePath.cpp | 52 ++++++++++++------ source/cppfs/source/LoginCredentials.cpp | 33 +++++++++--- source/cppfs/source/Tree.cpp | 43 ++++++++++----- source/cppfs/source/Url.cpp | 44 +++++++-------- source/cppfs/source/posix/LocalFileHandle.cpp | 5 ++ .../cppfs/source/posix/LocalFileIterator.cpp | 19 +++---- source/cppfs/source/posix/LocalFileSystem.cpp | 5 ++ source/cppfs/source/ssh/SshFileSystem.cpp | 19 +++++++ .../cppfs/source/ssh/SshInputStreamBuffer.cpp | 2 +- .../source/ssh/SshOutputStreamBuffer.cpp | 2 +- source/cppfs/source/system.cpp | 46 ++++++++++++---- 31 files changed, 517 insertions(+), 133 deletions(-) create mode 100644 source/cppfs/include/cppfs/units.h diff --git a/source/cppfs/CMakeLists.txt b/source/cppfs/CMakeLists.txt index 3bb7adf..61f7aa4 100644 --- a/source/cppfs/CMakeLists.txt +++ b/source/cppfs/CMakeLists.txt @@ -60,6 +60,7 @@ set(headers ${include_path}/Tree.h ${include_path}/Diff.h ${include_path}/Change.h + ${include_path}/units.h ${include_path}/${localfs}/LocalFileSystem.h ${include_path}/${localfs}/LocalFileHandle.h diff --git a/source/cppfs/include/cppfs/AbstractFileSystem.h b/source/cppfs/include/cppfs/AbstractFileSystem.h index a8a67d3..1b8e684 100644 --- a/source/cppfs/include/cppfs/AbstractFileSystem.h +++ b/source/cppfs/include/cppfs/AbstractFileSystem.h @@ -41,6 +41,15 @@ class CPPFS_API AbstractFileSystem * Path to file or directory */ virtual FileHandle open(const std::string & path) = 0; + + /** + * @brief + * Open file or directory in file system + * + * @param[in] path + * Path to file or directory + */ + virtual FileHandle open(std::string && path) = 0; }; diff --git a/source/cppfs/include/cppfs/Change.h b/source/cppfs/include/cppfs/Change.h index 5b0c830..23a9527 100644 --- a/source/cppfs/include/cppfs/Change.h +++ b/source/cppfs/include/cppfs/Change.h @@ -50,6 +50,17 @@ class CPPFS_API Change */ Change(Operation operation, const std::string & path); + /** + * @brief + * Constructor + * + * @param[in] operation + * Operation type + * @param[in] path + * Path on which the operation takes place + */ + Change(Operation operation, std::string && path); + /** * @brief * Copy constructor @@ -59,6 +70,15 @@ class CPPFS_API Change */ Change(const Change & change); + /** + * @brief + * Move constructor + * + * @param[in] change + * Other change + */ + Change(Change && change); + /** * @brief * Destructor @@ -74,6 +94,15 @@ class CPPFS_API Change */ Change & operator=(const Change & change); + /** + * @brief + * Move operator + * + * @param[in] change + * Other change + */ + Change & operator=(Change && change); + /** * @brief * Get string representation of the change @@ -99,7 +128,7 @@ class CPPFS_API Change * @return * Path on which the operation takes place */ - std::string path() const; + const std::string & path() const; protected: diff --git a/source/cppfs/include/cppfs/Diff.h b/source/cppfs/include/cppfs/Diff.h index 282d55e..6db8a93 100644 --- a/source/cppfs/include/cppfs/Diff.h +++ b/source/cppfs/include/cppfs/Diff.h @@ -3,6 +3,7 @@ #include +#include #include @@ -54,6 +55,15 @@ class CPPFS_API Diff */ void add(const Change & change); + /** + * @brief + * Add change + * + * @param[in] change + * Change + */ + void add(Change && change); + /** * @brief * Add change @@ -65,6 +75,26 @@ class CPPFS_API Diff */ void add(Change::Operation operation, const std::string & path); + /** + * @brief + * Add change + * + * @param[in] operation + * Operation type + * @param[in] path + * Path on which the operation takes place + */ + void add(Change::Operation operation, std::string && path); + + /** + * @brief + * Print changes to stream + * + * @param[in] stream + * The output stream + */ + void print(std::ostream & stream); + /** * @brief * Print changes to console diff --git a/source/cppfs/include/cppfs/FileHandle.h b/source/cppfs/include/cppfs/FileHandle.h index 5cc42b2..2c2012e 100644 --- a/source/cppfs/include/cppfs/FileHandle.h +++ b/source/cppfs/include/cppfs/FileHandle.h @@ -3,10 +3,10 @@ #include +#include #include #include #include -#include #include @@ -95,6 +95,15 @@ class CPPFS_API FileHandle */ FileHandle & operator=(const FileHandle & fileHandle); + /** + * @brief + * Move operator + * + * @param[in] fileHandle + * Source handle + */ + FileHandle & operator=(FileHandle && fileHandle); + /** * @brief * Get path diff --git a/source/cppfs/include/cppfs/FilePath.h b/source/cppfs/include/cppfs/FilePath.h index 0436a3e..7150686 100644 --- a/source/cppfs/include/cppfs/FilePath.h +++ b/source/cppfs/include/cppfs/FilePath.h @@ -136,6 +136,15 @@ class CPPFS_API FilePath */ void setPath(const std::string & path); + /** + * @brief + * Set path + * + * @param[in] path + * File path + */ + void setPath(std::string && path); + /** * @brief * Get native path as string @@ -189,7 +198,7 @@ class CPPFS_API FilePath * If you want trailing separators to remain in the string, use path(). * Calling this function triggers a full analysis of the path (costly operation). */ - std::string fullPath() const; + const std::string & fullPath() const; /** * @brief @@ -203,7 +212,7 @@ class CPPFS_API FilePath * "/path/to/something.ex/". * Calling this function triggers a full analysis of the path (costly operation). */ - std::string fileName() const; + const std::string & fileName() const; /** * @brief @@ -217,7 +226,7 @@ class CPPFS_API FilePath * "/path/to/something.ex/". * Calling this function triggers a full analysis of the path (costly operation). */ - std::string baseName() const; + const std::string & baseName() const; /** * @brief @@ -232,7 +241,7 @@ class CPPFS_API FilePath * string is returned. * Calling this function triggers a full analysis of the path (costly operation). */ - std::string extension() const; + const std::string & extension() const; /** * @brief @@ -246,7 +255,7 @@ class CPPFS_API FilePath * "/path/to/directory" and "/path/to/directory/". * Calling this function triggers a full analysis of the path (costly operation). */ - std::string directoryPath() const; + const std::string & directoryPath() const; /** * @brief @@ -259,7 +268,7 @@ class CPPFS_API FilePath * If there is no drive letter (Linux, Mac), an empty string is returned. * Calling this function triggers a full analysis of the path (costly operation). */ - std::string driveLetter() const; + const std::string & driveLetter() const; /** * @brief diff --git a/source/cppfs/include/cppfs/LoginCredentials.h b/source/cppfs/include/cppfs/LoginCredentials.h index 2a40dc4..173b0ea 100644 --- a/source/cppfs/include/cppfs/LoginCredentials.h +++ b/source/cppfs/include/cppfs/LoginCredentials.h @@ -77,7 +77,7 @@ class CPPFS_API LoginCredentials * @return * Reference to this value */ - LoginCredentials & operator=(const LoginCredentials && loginCredentials); + LoginCredentials & operator=(LoginCredentials && loginCredentials); /** * @brief @@ -125,7 +125,7 @@ class CPPFS_API LoginCredentials * @return * Value for key, "" if key does not exist */ - std::string value(const std::string & key) const; + const std::string & value(const std::string & key) const; /** * @brief @@ -138,6 +138,17 @@ class CPPFS_API LoginCredentials */ void setValue(const std::string & key, const std::string & value); + /** + * @brief + * Set value for key + * + * @param[in] key + * Key + * @param[in] value + * Value for key + */ + void setValue(const std::string & key, std::string && value); + protected: std::map m_values; ///< Key/value pairs diff --git a/source/cppfs/include/cppfs/Tree.h b/source/cppfs/include/cppfs/Tree.h index 489df7f..a804589 100644 --- a/source/cppfs/include/cppfs/Tree.h +++ b/source/cppfs/include/cppfs/Tree.h @@ -59,7 +59,7 @@ class CPPFS_API Tree * empty (""), the path of the first-order child elements * will equal their filenames, etc. */ - std::string path() const; + const std::string & path() const; /** * @brief @@ -70,6 +70,15 @@ class CPPFS_API Tree */ void setPath(const std::string & path); + /** + * @brief + * Set path + * + * @param[in] path + * Path to file or directory + */ + void setPath(std::string && path); + /** * @brief * Get filename @@ -77,7 +86,7 @@ class CPPFS_API Tree * @return * Filename */ - std::string fileName() const; + const std::string & fileName() const; /** * @brief @@ -88,6 +97,15 @@ class CPPFS_API Tree */ void setFileName(const std::string & filename); + /** + * @brief + * Set filename + * + * @param[in] filename + * Filename + */ + void setFileName(std::string && filename); + /** * @brief * Check if item is a file @@ -230,7 +248,7 @@ class CPPFS_API Tree * @return * SHA1 hash */ - std::string sha1() const; + const std::string & sha1() const; /** * @brief @@ -241,6 +259,15 @@ class CPPFS_API Tree */ void setSha1(const std::string & hash); + /** + * @brief + * Set sha1 hash + * + * @param[in] hash + * SHA1 hash + */ + void setSha1(std::string && hash); + /** * @brief * List files in directory diff --git a/source/cppfs/include/cppfs/Url.h b/source/cppfs/include/cppfs/Url.h index f6799c0..0f902e4 100644 --- a/source/cppfs/include/cppfs/Url.h +++ b/source/cppfs/include/cppfs/Url.h @@ -42,7 +42,7 @@ class CPPFS_API Url * @param[in] url * Url to move */ - Url(const Url && url); + Url(Url && url); /** * @brief @@ -60,7 +60,7 @@ class CPPFS_API Url * @param[in] url * Url */ - Url(const std::string && url); + Url(std::string && url); /** * @brief @@ -99,7 +99,7 @@ class CPPFS_API Url * @return * Reference to this value */ - Url & operator=(const Url && url); + Url & operator=(Url && url); /** * @brief @@ -117,7 +117,7 @@ class CPPFS_API Url * @return * Protocol part (including "://") */ - std::string protocol() const; + const std::string & protocol() const; /** * @brief @@ -126,7 +126,7 @@ class CPPFS_API Url * @return * Location (everything after the protocol part) */ - std::string location() const; + const std::string & location() const; /** * @brief @@ -135,7 +135,7 @@ class CPPFS_API Url * @return * Address (including username, password, and hostname) */ - std::string address() const; + const std::string & address() const; /** * @brief @@ -144,7 +144,7 @@ class CPPFS_API Url * @return * Path (everything after the address) */ - std::string path() const; + const std::string & path() const; /** * @brief @@ -153,7 +153,7 @@ class CPPFS_API Url * @return * Login part (username and password) */ - std::string login() const; + const std::string & login() const; /** * @brief @@ -162,7 +162,7 @@ class CPPFS_API Url * @return * Host name */ - std::string host() const; + const std::string & host() const; /** * @brief @@ -171,7 +171,7 @@ class CPPFS_API Url * @return * User name */ - std::string username() const; + const std::string & username() const; /** * @brief @@ -180,7 +180,7 @@ class CPPFS_API Url * @return * Password */ - std::string password() const; + const std::string & password() const; protected: diff --git a/source/cppfs/include/cppfs/posix/LocalFileHandle.h b/source/cppfs/include/cppfs/posix/LocalFileHandle.h index 65ef74e..ad133ce 100644 --- a/source/cppfs/include/cppfs/posix/LocalFileHandle.h +++ b/source/cppfs/include/cppfs/posix/LocalFileHandle.h @@ -32,6 +32,17 @@ class CPPFS_API LocalFileHandle : public AbstractFileHandleBackend */ LocalFileHandle(std::shared_ptr fs, const std::string & path); + /** + * @brief + * Constructor + * + * @param[in] fs + * File system that created this handle + * @param[in] path + * Path to file or directory + */ + LocalFileHandle(std::shared_ptr fs, std::string && path); + /** * @brief * Destructor diff --git a/source/cppfs/include/cppfs/posix/LocalFileIterator.h b/source/cppfs/include/cppfs/posix/LocalFileIterator.h index 485df68..59da085 100644 --- a/source/cppfs/include/cppfs/posix/LocalFileIterator.h +++ b/source/cppfs/include/cppfs/posix/LocalFileIterator.h @@ -34,6 +34,17 @@ class CPPFS_API LocalFileIterator : public AbstractFileIteratorBackend */ LocalFileIterator(std::shared_ptr fs, const std::string & path); + /** + * @brief + * Constructor + * + * @param[in] fs + * File system that created this iterator + * @param[in] path + * Path to file or directory + */ + LocalFileIterator(std::shared_ptr fs, std::string && path); + /** * @brief * Destructor diff --git a/source/cppfs/include/cppfs/posix/LocalFileSystem.h b/source/cppfs/include/cppfs/posix/LocalFileSystem.h index 3c137e0..df75723 100644 --- a/source/cppfs/include/cppfs/posix/LocalFileSystem.h +++ b/source/cppfs/include/cppfs/posix/LocalFileSystem.h @@ -32,6 +32,7 @@ class CPPFS_API LocalFileSystem : public AbstractFileSystem, public std::enable_ // Virtual AbstractFileSystem functions virtual FileHandle open(const std::string & path) override; + virtual FileHandle open(std::string && path) override; }; diff --git a/source/cppfs/include/cppfs/ssh/SshFileSystem.h b/source/cppfs/include/cppfs/ssh/SshFileSystem.h index 3a5c7f6..7735efd 100644 --- a/source/cppfs/include/cppfs/ssh/SshFileSystem.h +++ b/source/cppfs/include/cppfs/ssh/SshFileSystem.h @@ -51,6 +51,32 @@ class CPPFS_API SshFileSystem : public AbstractFileSystem, public std::enable_sh const std::string & privateKey ); + /** + * @brief + * Constructor + * + * @param[in] host + * Host name + * @param[in] port + * Port + * @param[in] username + * User name + * @param[in] password + * Password + * @param[in] publicKey + * Path to public key file + * @param[in] privateKey + * Path to private key file + */ + SshFileSystem( + std::string && host, + int port, + std::string && username, + std::string && password, + std::string && publicKey, + std::string && privateKey + ); + /** * @brief * Destructor @@ -59,6 +85,7 @@ class CPPFS_API SshFileSystem : public AbstractFileSystem, public std::enable_sh // Virtual AbstractFileSystem functions virtual FileHandle open(const std::string & path) override; + virtual FileHandle open(std::string && path) override; protected: diff --git a/source/cppfs/include/cppfs/ssh/SshInputStreamBuffer.h b/source/cppfs/include/cppfs/ssh/SshInputStreamBuffer.h index c51f663..6f7cd07 100644 --- a/source/cppfs/include/cppfs/ssh/SshInputStreamBuffer.h +++ b/source/cppfs/include/cppfs/ssh/SshInputStreamBuffer.h @@ -8,6 +8,7 @@ #include #include +#include namespace cppfs @@ -39,7 +40,7 @@ class CPPFS_API SshInputStreamBuffer : public std::streambuf * @param[n] putbackSize * Size of the putback area */ - SshInputStreamBuffer(std::shared_ptr fs, const std::string & path, std::ios_base::openmode mode, size_t bufferSize = 32 * 1024, size_t putBackSize = 128); + SshInputStreamBuffer(std::shared_ptr fs, const std::string & path, std::ios_base::openmode mode, size_t bufferSize = 32_kB, size_t putBackSize = 128_B); /** * @brief @@ -55,7 +56,7 @@ class CPPFS_API SshInputStreamBuffer : public std::streambuf protected: std::shared_ptr m_fs; ///< File system that created this iterator - std::string m_path; ///< Path to file or directory + const std::string m_path; ///< Path to file or directory void * m_file; ///< SFTP file handle const size_t m_putbackSize; ///< Size of the putback area std::vector m_buffer; ///< Read buffer diff --git a/source/cppfs/include/cppfs/ssh/SshOutputStreamBuffer.h b/source/cppfs/include/cppfs/ssh/SshOutputStreamBuffer.h index 3b6f619..16c6023 100644 --- a/source/cppfs/include/cppfs/ssh/SshOutputStreamBuffer.h +++ b/source/cppfs/include/cppfs/ssh/SshOutputStreamBuffer.h @@ -8,6 +8,7 @@ #include #include +#include namespace cppfs @@ -44,7 +45,7 @@ class CPPFS_API SshOutputStreamBuffer : public std::streambuf * * See https://c-ares.haxx.se/mail/libssh2-devel-archive-2010-02/0170.shtml */ - SshOutputStreamBuffer(std::shared_ptr fs, const std::string & path, std::ios_base::openmode mode, size_t bufferSize = 24 * 1024); + SshOutputStreamBuffer(std::shared_ptr fs, const std::string & path, std::ios_base::openmode mode, size_t bufferSize = 24_kB); /** * @brief @@ -60,10 +61,10 @@ class CPPFS_API SshOutputStreamBuffer : public std::streambuf protected: - std::shared_ptr m_fs; ///< File system that created this iterator - std::string m_path; ///< Path to file or directory - void * m_file; ///< SFTP file handle - std::vector m_buffer; ///< Write buffer + std::shared_ptr m_fs; ///< File system that created this iterator + const std::string m_path; ///< Path to file or directory + void * m_file; ///< SFTP file handle + std::vector m_buffer; ///< Write buffer }; diff --git a/source/cppfs/include/cppfs/system.h b/source/cppfs/include/cppfs/system.h index ad65c25..1e3ad4c 100644 --- a/source/cppfs/include/cppfs/system.h +++ b/source/cppfs/include/cppfs/system.h @@ -25,8 +25,12 @@ namespace system * * @return * Home directory (native path) +* +* @remarks +* It is assumed that the home directory doesn't change +* for the process lifetime. */ -CPPFS_API std::string homeDir(); +CPPFS_API const std::string & homeDir(); /** * @brief @@ -37,6 +41,10 @@ CPPFS_API std::string homeDir(); * * @return * Config directory (native path) +* +* @remarks +* It is assumed that the config directory doesn't change +* for the process lifetime. */ CPPFS_API std::string configDir(const std::string & application); diff --git a/source/cppfs/include/cppfs/units.h b/source/cppfs/include/cppfs/units.h new file mode 100644 index 0000000..5c2b860 --- /dev/null +++ b/source/cppfs/include/cppfs/units.h @@ -0,0 +1,35 @@ + +#pragma once + + +namespace cppfs +{ + + +inline unsigned long long operator""_B(unsigned long long value) +{ + return value; +} + +inline unsigned long long operator""_kB(unsigned long long value) +{ + return value * 1024; +} + +inline unsigned long long operator""_MB(unsigned long long value) +{ + return value * 1024 * 1024; +} + +inline unsigned long long operator""_GB(unsigned long long value) +{ + return value * 1024 * 1024 * 1024; +} + +inline unsigned long long operator""_TB(unsigned long long value) +{ + return value * 1024 * 1024 * 1024 * 1024; +} + + +} // namespace cppfs diff --git a/source/cppfs/source/Change.cpp b/source/cppfs/source/Change.cpp index 25f804e..9d74d0e 100644 --- a/source/cppfs/source/Change.cpp +++ b/source/cppfs/source/Change.cpp @@ -18,12 +18,24 @@ Change::Change(Operation operation, const std::string & path) { } +Change::Change(Operation operation, std::string && path) +: m_operation(operation) +, m_path(std::move(path)) +{ +} + Change::Change(const Change & change) : m_operation(change.m_operation) , m_path(change.m_path) { } +Change::Change(Change && change) +: m_operation(std::move(change.m_operation)) +, m_path(std::move(change.m_path)) +{ +} + Change::~Change() { } @@ -36,6 +48,14 @@ Change & Change::operator=(const Change & change) return *this; } +Change & Change::operator=(Change && change) +{ + m_operation = std::move(change.m_operation); + m_path = std::move(change.m_path); + + return *this; +} + std::string Change::toString() const { switch (m_operation) @@ -53,7 +73,7 @@ Change::Operation Change::operation() const return m_operation; } -std::string Change::path() const +const std::string & Change::path() const { return m_path; } diff --git a/source/cppfs/source/Diff.cpp b/source/cppfs/source/Diff.cpp index cb21cb7..c3043dc 100644 --- a/source/cppfs/source/Diff.cpp +++ b/source/cppfs/source/Diff.cpp @@ -31,18 +31,33 @@ void Diff::add(const Change & change) m_changes.push_back(change); } +void Diff::add(Change && change) +{ + m_changes.push_back(change); +} + void Diff::add(Change::Operation operation, const std::string & path) { - m_changes.push_back(Change(operation, path)); + m_changes.emplace_back(operation, path); } -void Diff::print() +void Diff::add(Change::Operation operation, std::string && path) +{ + m_changes.emplace_back(operation, path); +} + +void Diff::print(std::ostream & stream) { for (size_t i = 0; i < m_changes.size(); i++) { - std::cout << m_changes[i].toString() << std::endl; + stream << m_changes[i].toString() << std::endl; } } +void Diff::print() +{ + print(std::cout); +} + } // namespace cppfs diff --git a/source/cppfs/source/FileHandle.cpp b/source/cppfs/source/FileHandle.cpp index 90fb06f..36f989f 100644 --- a/source/cppfs/source/FileHandle.cpp +++ b/source/cppfs/source/FileHandle.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #if defined(__APPLE__) #define COMMON_DIGEST_FOR_OPENSSL @@ -60,6 +61,13 @@ FileHandle & FileHandle::operator=(const FileHandle & fileHandle) return *this; } +FileHandle & FileHandle::operator=(FileHandle && fileHandle) +{ + m_backend = std::move(fileHandle.m_backend); + + return *this; +} + std::string FileHandle::path() const { return m_backend ? m_backend->path() : ""; @@ -142,7 +150,8 @@ void FileHandle::traverse(FileVisitor & visitor) Tree * FileHandle::readTree(const std::string & path, bool includeHash) const { // Check if file or directory exists - if (!exists()) { + if (!exists()) + { return nullptr; } @@ -157,7 +166,9 @@ Tree * FileHandle::readTree(const std::string & path, bool includeHash) const tree->setUserId(userId()); tree->setGroupId(groupId()); tree->setPermissions(permissions()); - if (includeHash) { + + if (includeHash) + { tree->setSha1(sha1()); } @@ -249,13 +260,15 @@ void FileHandle::setPermissions(unsigned long permissions) std::string FileHandle::sha1() const { // Check file - if (!isFile()) { + if (!isFile()) + { return ""; } // Open file std::istream * inputStream = createInputStream(); - if (!inputStream) { + if (!inputStream) + { return ""; } @@ -268,17 +281,15 @@ std::string FileHandle::sha1() const while (!inputStream->eof()) { // Read a maximum of 1024 bytes at once - int size = 1024; - // Read data from file - char buf[1024]; - inputStream->read(buf, size); + std::array buf; + inputStream->read(buf.data(), buf.size()); size_t count = inputStream->gcount(); if (count > 0) { // Update hash - SHA1_Update(&context, buf, count); + SHA1_Update(&context, buf.data(), count); } else break; } @@ -293,13 +304,16 @@ std::string FileHandle::sha1() const std::string FileHandle::base64() const { // Check file - if (!isFile()) { + if (!isFile()) + { return ""; } // Open file std::istream * inputStream = createInputStream(); - if (!inputStream) { + + if (!inputStream) + { return ""; } @@ -445,7 +459,8 @@ bool FileHandle::copy(FileHandle & dest) } // Otherwise, use generic (slow) method - else { + else + { return genericCopy(dest); } } @@ -467,7 +482,8 @@ bool FileHandle::move(FileHandle & dest) } // Otherwise, use generic (slow) method - else { + else + { return genericMove(dest); } } @@ -489,7 +505,8 @@ bool FileHandle::createLink(FileHandle & dest) } // Otherwise, this is not possible - else { + else + { return false; } } @@ -511,7 +528,8 @@ bool FileHandle::createSymbolicLink(FileHandle & dest) } // Otherwise, this is not possible - else { + else + { return false; } } @@ -643,11 +661,12 @@ bool FileHandle::genericCopy(FileHandle & dest) // Open files std::istream * in = createInputStream(std::ios::binary); std::ostream * out = dest.createOutputStream(std::ios::binary | std::ios::trunc); + if (!in || !out) { // Clean up streams - if (in) delete in; - if (out) delete out; + delete in; + delete out; // Error! return false; diff --git a/source/cppfs/source/FilePath.cpp b/source/cppfs/source/FilePath.cpp index 1fb572d..bb09850 100644 --- a/source/cppfs/source/FilePath.cpp +++ b/source/cppfs/source/FilePath.cpp @@ -135,6 +135,33 @@ void FilePath::setPath(const std::string & path) } } +void FilePath::setPath(std::string && path) +{ + // Set new path + m_path = std::move(path); + + // Reset state + m_pointsToContent = false; + m_details = false; + m_fullPath = ""; + m_filename = ""; + m_basename = ""; + m_extension = ""; + m_directoryPath = ""; + m_driveLetter = ""; + m_absolute = false; + + // Convert path into unified form + std::replace(m_path.begin(), m_path.end(), '\\', '/'); + + // Check if path ends with a delimiter + auto pos = m_path.find_last_of('/'); + if (pos == m_path.size()-1) + { + m_pointsToContent = true; + } +} + std::string FilePath::toNative() const { auto path = m_path; @@ -156,42 +183,42 @@ bool FilePath::pointsToContent() const return m_pointsToContent; } -std::string FilePath::fullPath() const +const std::string & FilePath::fullPath() const { analyze(); return m_fullPath; } -std::string FilePath::fileName() const +const std::string & FilePath::fileName() const { analyze(); return m_filename; } -std::string FilePath::baseName() const +const std::string & FilePath::baseName() const { analyze(); return m_basename; } -std::string FilePath::extension() const +const std::string & FilePath::extension() const { analyze(); return m_extension; } -std::string FilePath::directoryPath() const +const std::string & FilePath::directoryPath() const { analyze(); return m_directoryPath; } -std::string FilePath::driveLetter() const +const std::string & FilePath::driveLetter() const { analyze(); @@ -257,7 +284,7 @@ std::string FilePath::resolved() const for (size_t i = 0; i < numParts; i++) { // Get sub-path - std::string path = parts[i]; + const std::string & path = parts[i]; // Check if it is the beginning of an absolute path if (i == 0 && (path.empty() || (path.length() == 2 && path[1] == ':'))) { @@ -341,7 +368,7 @@ void FilePath::analyze() const for (size_t i = 0; i < numParts; i++) { // Get sub-path - std::string path = parts[i]; + const std::string & path = parts[i]; // If this is the first path and it is absolute, ensure '/' at the end if (i == 0 && (path.empty() || (path.length() == 2 && path[1] == ':'))) @@ -405,14 +432,7 @@ void FilePath::analyze() const // Determine drive letter pos = m_fullPath.find_first_of(':'); - if (pos == 1) - { - m_driveLetter = m_path.substr(0, pos+1); - } - else - { - m_driveLetter = ""; - } + m_driveLetter = pos == 1 ? m_path.substr(0, pos+1) : ""; // Set state m_details = true; diff --git a/source/cppfs/source/LoginCredentials.cpp b/source/cppfs/source/LoginCredentials.cpp index 053048c..7eefea8 100644 --- a/source/cppfs/source/LoginCredentials.cpp +++ b/source/cppfs/source/LoginCredentials.cpp @@ -9,6 +9,16 @@ #include +namespace +{ + + +const std::string emptyValue = ""; + + +} // namespace + + namespace cppfs { @@ -23,7 +33,7 @@ LoginCredentials::LoginCredentials(const LoginCredentials & loginCredentials) } LoginCredentials::LoginCredentials(const LoginCredentials && loginCredentials) -: m_values(loginCredentials.m_values) + : m_values(std::move(loginCredentials.m_values)) { } @@ -38,9 +48,9 @@ LoginCredentials & LoginCredentials::operator=(const LoginCredentials & loginCre return *this; } -LoginCredentials & LoginCredentials::operator=(const LoginCredentials && loginCredentials) +LoginCredentials & LoginCredentials::operator=(LoginCredentials && loginCredentials) { - m_values = loginCredentials.m_values; + m_values = std::move(loginCredentials.m_values); return *this; } @@ -72,6 +82,7 @@ bool LoginCredentials::load(const std::string & path) // Close delete in; + return true; } @@ -87,8 +98,8 @@ bool LoginCredentials::save(const std::string & path) const // Get all keys and values for (auto it : m_values) { - std::string key = it.first; - std::string value = it.second; + const std::string & key = it.first; + const std::string & value = it.second; if (!key.empty() && !value.empty()) { @@ -98,6 +109,7 @@ bool LoginCredentials::save(const std::string & path) const // Close delete out; + return true; } @@ -106,16 +118,16 @@ bool LoginCredentials::isSet(const std::string & key) const return (m_values.find(key) != m_values.end()); } -std::string LoginCredentials::value(const std::string & key) const +const std::string & LoginCredentials::value(const std::string & key) const { - auto it = m_values.find(key); + const auto it = m_values.find(key); if (it != m_values.end()) { return it->second; } - return ""; + return emptyValue; } void LoginCredentials::setValue(const std::string & key, const std::string & value) @@ -123,5 +135,10 @@ void LoginCredentials::setValue(const std::string & key, const std::string & val m_values[key] = value; } +void LoginCredentials::setValue(const std::string & key, std::string && value) +{ + m_values[key] = std::move(value); +} + } // namespace cppfs diff --git a/source/cppfs/source/Tree.cpp b/source/cppfs/source/Tree.cpp index 551acf2..4f4335b 100644 --- a/source/cppfs/source/Tree.cpp +++ b/source/cppfs/source/Tree.cpp @@ -44,7 +44,7 @@ void Tree::clear() m_children.clear(); } -std::string Tree::path() const +const std::string & Tree::path() const { return m_path; } @@ -54,7 +54,12 @@ void Tree::setPath(const std::string & path) m_path = path; } -std::string Tree::fileName() const +void Tree::setPath(std::string && path) +{ + m_path = std::move(path); +} + +const std::string & Tree::fileName() const { return m_filename; } @@ -64,6 +69,11 @@ void Tree::setFileName(const std::string & filename) m_filename = filename; } +void Tree::setFileName(std::string && filename) +{ + m_filename = filename; +} + bool Tree::isFile() const { return !m_directory; @@ -139,7 +149,7 @@ void Tree::setPermissions(unsigned int permissions) m_permissions = permissions; } -std::string Tree::sha1() const +const std::string & Tree::sha1() const { return m_sha1; } @@ -149,11 +159,17 @@ void Tree::setSha1(const std::string & hash) m_sha1 = hash; } +void Tree::setSha1(std::string && hash) +{ + m_sha1 = std::move(hash); +} + std::vector Tree::listFiles() const { std::vector children; - if (!m_directory) { + if (!m_directory) + { return children; } @@ -178,7 +194,8 @@ std::vector & Tree::children() void Tree::add(Tree * tree) { // Check parameters - if (!tree) { + if (!tree) + { return; } @@ -189,12 +206,13 @@ void Tree::add(Tree * tree) void Tree::remove(Tree * tree) { // Check parameters - if (!tree) { + if (!tree) + { return; } // Find item in list - auto it = std::find(m_children.begin(), m_children.end(), tree); + const auto it = std::find(m_children.begin(), m_children.end(), tree); if (it != m_children.end()) { // Remove from list @@ -239,7 +257,8 @@ void Tree::fromVariant(const cppexpose::Variant & obj) clear(); // Check object - if (!obj.isVariantMap()) { + if (!obj.isVariantMap()) + { return; } @@ -257,7 +276,7 @@ void Tree::fromVariant(const cppexpose::Variant & obj) m_sha1 = map.at("sha1").toString(); // Read children - auto it = map.find("children"); + const auto it = map.find("children"); if (it != map.end()) { const cppexpose::Variant & children = (*it).second; @@ -320,7 +339,7 @@ void Tree::createDiff(const Tree * currentState, const Tree * targetState, Diff auto currentFiles = currentState->listFiles(); // Delete files which are in the current state but not in the target state - for (auto * file : currentState->children()) + for (const auto * file : currentState->children()) { // Check if file is in the target state if (std::find(targetFiles.begin(), targetFiles.end(), file->fileName()) == targetFiles.end()) @@ -341,10 +360,10 @@ void Tree::createDiff(const Tree * currentState, const Tree * targetState, Diff } // Copy files which are new or changed - for (auto * targetFile : targetState->children()) + for (const auto * targetFile : targetState->children()) { // Get file in the current state - auto it = std::find(currentFiles.begin(), currentFiles.end(), targetFile->fileName()); + const auto it = std::find(currentFiles.begin(), currentFiles.end(), targetFile->fileName()); Tree * currentFile = (it != currentFiles.end()) ? currentState->children()[std::distance(currentFiles.begin(), it)] : nullptr; // Directory diff --git a/source/cppfs/source/Url.cpp b/source/cppfs/source/Url.cpp index b64aee9..1c9b5a7 100644 --- a/source/cppfs/source/Url.cpp +++ b/source/cppfs/source/Url.cpp @@ -36,17 +36,17 @@ Url::Url(const Url & url) { } -Url::Url(const Url && url) -: m_url(url.m_url) -, m_analyzed(url.m_analyzed) -, m_protocol(url.m_protocol) -, m_location(url.m_location) -, m_address(url.m_address) -, m_path(url.m_path) -, m_login(url.m_login) -, m_host(url.m_host) -, m_username(url.m_username) -, m_password(url.m_password) +Url::Url(Url && url) +: m_url(std::move(url.m_url)) +, m_analyzed(std::move(url.m_analyzed)) +, m_protocol(std::move(url.m_protocol)) +, m_location(std::move(url.m_location)) +, m_address(std::move(url.m_address)) +, m_path(std::move(url.m_path)) +, m_login(std::move(url.m_login)) +, m_host(std::move(url.m_host)) +, m_username(std::move(url.m_username)) +, m_password(std::move(url.m_password)) { } @@ -64,8 +64,8 @@ Url::Url(const std::string & url) { } -Url::Url(const std::string && url) -: m_url(url) +Url::Url(std::string && url) +: m_url(std::move(url)) , m_analyzed(false) , m_protocol("") , m_location("") @@ -112,7 +112,7 @@ Url & Url::operator=(const Url & url) return *this; } -Url & Url::operator=(const Url && url) +Url & Url::operator=(Url && url) { m_url = std::move(url.m_url); m_analyzed = std::move(url.m_analyzed); @@ -133,49 +133,49 @@ const std::string & Url::toString() const return m_url; } -std::string Url::protocol() const +const std::string & Url::protocol() const { analyze(); return m_protocol; } -std::string Url::location() const +const std::string & Url::location() const { analyze(); return m_location; } -std::string Url::address() const +const std::string & Url::address() const { analyze(); return m_address; } -std::string Url::path() const +const std::string & Url::path() const { analyze(); return m_path; } -std::string Url::login() const +const std::string & Url::login() const { analyze(); return m_login; } -std::string Url::host() const +const std::string & Url::host() const { analyze(); return m_host; } -std::string Url::username() const +const std::string & Url::username() const { analyze(); return m_username; } -std::string Url::password() const +const std::string & Url::password() const { analyze(); return m_password; diff --git a/source/cppfs/source/posix/LocalFileHandle.cpp b/source/cppfs/source/posix/LocalFileHandle.cpp index aa40b5a..3c554a7 100644 --- a/source/cppfs/source/posix/LocalFileHandle.cpp +++ b/source/cppfs/source/posix/LocalFileHandle.cpp @@ -18,6 +18,11 @@ namespace cppfs LocalFileHandle::LocalFileHandle(std::shared_ptr fs, const std::string & path) +: LocalFileHandle(fs, std::string(path)) +{ +} + +LocalFileHandle::LocalFileHandle(std::shared_ptr fs, std::string && path) : m_fs(fs) , m_path(path) , m_fileInfo(nullptr) diff --git a/source/cppfs/source/posix/LocalFileIterator.cpp b/source/cppfs/source/posix/LocalFileIterator.cpp index dc6b3d8..6bf222d 100644 --- a/source/cppfs/source/posix/LocalFileIterator.cpp +++ b/source/cppfs/source/posix/LocalFileIterator.cpp @@ -12,6 +12,11 @@ namespace cppfs LocalFileIterator::LocalFileIterator(std::shared_ptr fs, const std::string & path) +: LocalFileIterator(fs, std::string(path)) +{ +} + +LocalFileIterator::LocalFileIterator(std::shared_ptr fs, std::string && path) : m_fs(fs) , m_path(path) , m_dir(nullptr) @@ -73,8 +78,8 @@ std::string LocalFileIterator::name() const return ""; } - // Return filename of current item - return std::string(m_entry->d_name); + // Return filename of current item but it's a bad idea to construct an std::string from a nullptr + return m_entry->d_name ? std::string(m_entry->d_name) : std::string(); } void LocalFileIterator::next() @@ -90,19 +95,15 @@ void LocalFileIterator::readNextEntry() // Read next entry m_entry = readdir(m_dir); m_index++; - if (!m_entry) return; + if (!m_entry || !m_entry->d_name) return; // Omit '.' and '..' std::string name = m_entry->d_name; - while (m_entry && (name == ".." || name == ".")) + while (m_entry && m_entry->d_name && (name == ".." || name == ".")) { m_entry = readdir(m_dir); - if (m_entry) { - name = m_entry->d_name; - } else { - name = ""; - } + name = m_entry && m_entry->d_name ? std::string(m_entry->d_name) : std::string(); } } diff --git a/source/cppfs/source/posix/LocalFileSystem.cpp b/source/cppfs/source/posix/LocalFileSystem.cpp index 3a8ad71..83ad5b8 100644 --- a/source/cppfs/source/posix/LocalFileSystem.cpp +++ b/source/cppfs/source/posix/LocalFileSystem.cpp @@ -18,6 +18,11 @@ LocalFileSystem::~LocalFileSystem() } FileHandle LocalFileSystem::open(const std::string & path) +{ + return open(std::string(path)); +} + +FileHandle LocalFileSystem::open(std::string && path) { return FileHandle(new LocalFileHandle(shared_from_this(), path)); } diff --git a/source/cppfs/source/ssh/SshFileSystem.cpp b/source/cppfs/source/ssh/SshFileSystem.cpp index 1d5a7af..cffb958 100644 --- a/source/cppfs/source/ssh/SshFileSystem.cpp +++ b/source/cppfs/source/ssh/SshFileSystem.cpp @@ -42,12 +42,31 @@ SshFileSystem::SshFileSystem(const std::string & host, int port, const std::stri connect(); } +SshFileSystem::SshFileSystem(std::string && host, int port, std::string && username, std::string && password, std::string && publicKey, std::string && privateKey) +: m_host(std::move(host)) +, m_port(std::move(port)) +, m_username(std::move(username)) +, m_password(std::move(password)) +, m_publicKey(std::move(publicKey)) +, m_privateKey(std::move(privateKey)) +, m_socket(0) +, m_session(nullptr) +, m_sftpSession(nullptr) +{ + connect(); +} + SshFileSystem::~SshFileSystem() { disconnect(); } FileHandle SshFileSystem::open(const std::string & path) +{ + return open(std::string(path)); +} + +FileHandle SshFileSystem::open(std::string && path) { return FileHandle(new SshFileHandle(shared_from_this(), path)); } diff --git a/source/cppfs/source/ssh/SshInputStreamBuffer.cpp b/source/cppfs/source/ssh/SshInputStreamBuffer.cpp index a7cd959..5488700 100644 --- a/source/cppfs/source/ssh/SshInputStreamBuffer.cpp +++ b/source/cppfs/source/ssh/SshInputStreamBuffer.cpp @@ -12,7 +12,7 @@ #ifdef max - #undef max + #undef max #endif diff --git a/source/cppfs/source/ssh/SshOutputStreamBuffer.cpp b/source/cppfs/source/ssh/SshOutputStreamBuffer.cpp index df9c2db..daca34c 100644 --- a/source/cppfs/source/ssh/SshOutputStreamBuffer.cpp +++ b/source/cppfs/source/ssh/SshOutputStreamBuffer.cpp @@ -98,7 +98,7 @@ int SshOutputStreamBuffer::sync() if (size > 0) { // Write to stream - auto res = libssh2_sftp_write((LIBSSH2_SFTP_HANDLE *)m_file, &m_buffer.front(), size); + /*auto res = */libssh2_sftp_write((LIBSSH2_SFTP_HANDLE *)m_file, &m_buffer.front(), size); // [TODO] Handle errors /* diff --git a/source/cppfs/source/system.cpp b/source/cppfs/source/system.cpp index e4c62e2..b230e37 100644 --- a/source/cppfs/source/system.cpp +++ b/source/cppfs/source/system.cpp @@ -4,32 +4,56 @@ #include -namespace cppfs -{ -namespace system +namespace { - -std::string homeDir() +std::string obtainHomeDir() { #if defined(SYSTEM_WINDOWS) - return std::string(getenv("HOMEPATH")); + return std::string(getenv("HOMEPATH")); #else - return std::string(getenv("HOME")); + return std::string(getenv("HOME")); #endif } -std::string configDir(const std::string & application) +std::string obtainConfigDir() { #if defined(SYSTEM_WINDOWS) - return std::string(getenv("APPDATA")) + "\\" + application; + return std::string(getenv("APPDATA")) + "\\"; #elif defined(SYSTEM_DARWIN) - return std::string(getenv("HOME")) + "/Library/Preferences/" + application; + return std::string(getenv("HOME")) + "/Library/Preferences/"; #else - return std::string(getenv("HOME")) + "/.config/" + application; + return std::string(getenv("HOME")) + "/.config/"; #endif } +} // namespace + + +namespace cppfs +{ + + +namespace system +{ + + +const std::string & homeDir() +{ + static const std::string dir = obtainHomeDir(); + + return dir; +} + +std::string configDir(const std::string & application) +{ + static const std::string dir = obtainConfigDir(); + + return dir + application; +} + } // namespace system + + } // namespace cppfs