From 9089b8555f53448f444cb7366cb0cfa89e47b869 Mon Sep 17 00:00:00 2001 From: Valient Gough Date: Fri, 25 Aug 2017 22:34:33 -0700 Subject: [PATCH] cleanup lint warnings, run clang-format --- CMakeLists.txt | 16 +++++++++++++- encfs/BlockFileIO.cpp | 18 ++++++++++------ encfs/BlockNameIO.cpp | 6 ++++-- encfs/CipherFileIO.cpp | 49 ++++++++++++++++++++++++------------------ encfs/Context.cpp | 7 +++--- encfs/Context.h | 4 ++-- encfs/DirNode.cpp | 2 +- encfs/FileNode.cpp | 4 ++-- encfs/MACFileIO.cpp | 6 +++--- encfs/RawFileIO.cpp | 24 ++++++++++++--------- encfs/SSL_Cipher.cpp | 12 ++++++----- encfs/XmlReader.cpp | 2 +- encfs/encfs.cpp | 22 +++++++++++-------- 13 files changed, 105 insertions(+), 67 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 76e617a..f3cd724 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -168,7 +168,21 @@ if (${CMAKE_MAJOR_VERSION}.${CMAKE_MINOR_VERSION} GREATER 3.5) # Need 3.6 or abo message(STATUS "clang-tidy not found.") else() message(STATUS "clang-tidy found: ${CLANG_TIDY_EXE}") - set(DO_CLANG_TIDY "${CLANG_TIDY_EXE}" "-checks=*,-modernize-loop-convert,-cppcoreguidelines-pro-*,-readability-inconsistent-declaration-parameter-name,-google-readability-casting,-cert-err58-cpp,-google-runtime-int,-readability-named-parameter,-google-build-using-namespace,-misc-unused-parameters,-google-runtime-references") + string(CONCAT TIDY_OPTS "-checks=*" + ",-cert-err58-cpp" + ",-cppcoreguidelines-pro-*" + ",-google-build-using-namespace" + ",-google-readability-casting" + ",-google-readability-todo" + ",-google-runtime-int" + ",-google-runtime-references" + ",-misc-misplaced-widening-cast" + ",-misc-unused-parameters" + ",-modernize-loop-convert" + ",-readability-inconsistent-declaration-parameter-name" + ",-readability-named-parameter" + ) + set(DO_CLANG_TIDY "${CLANG_TIDY_EXE}" ${TIDY_OPTS}) #set(DO_CLANG_TIDY "${CLANG_TIDY_EXE}" "-fix" "-checks=-*,google-readability-redundant-smartptr-get") endif() else() diff --git a/encfs/BlockFileIO.cpp b/encfs/BlockFileIO.cpp index 5cf323c..b0eede1 100644 --- a/encfs/BlockFileIO.cpp +++ b/encfs/BlockFileIO.cpp @@ -121,7 +121,8 @@ ssize_t BlockFileIO::cacheWriteOneBlock(const IORequest &req) { ssize_t BlockFileIO::read(const IORequest &req) const { CHECK(_blockSize != 0); - int partialOffset = req.offset % _blockSize; //can be int as _blockSize is int + int partialOffset = + req.offset % _blockSize; // can be int as _blockSize is int off_t blockNum = req.offset / _blockSize; ssize_t result = 0; @@ -159,7 +160,9 @@ ssize_t BlockFileIO::read(const IORequest &req) const { result = readSize; break; } - if (readSize <= partialOffset) break; // didn't get enough bytes + if (readSize <= partialOffset) { + break; // didn't get enough bytes + } size_t cpySize = min((size_t)(readSize - partialOffset), size); CHECK(cpySize <= readSize); @@ -200,7 +203,8 @@ ssize_t BlockFileIO::write(const IORequest &req) { // where write request begins off_t blockNum = req.offset / _blockSize; - int partialOffset = req.offset % _blockSize; //can be int as _blockSize is int + int partialOffset = + req.offset % _blockSize; // can be int as _blockSize is int // last block of file (for testing write overlaps with file boundary) off_t lastFileBlock = fileSize / _blockSize; @@ -267,7 +271,7 @@ ssize_t BlockFileIO::write(const IORequest &req) { if (blockNum > lastNonEmptyBlock) { // just pad.. - blockReq.dataLen = toCopy + partialOffset; + blockReq.dataLen = partialOffset + toCopy; } else { // have to merge with existing block data.. blockReq.dataLen = _blockSize; @@ -318,7 +322,7 @@ int BlockFileIO::blockSize() const { return _blockSize; } int BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) { off_t oldLastBlock = oldSize / _blockSize; off_t newLastBlock = newSize / _blockSize; - int newBlockSize = newSize % _blockSize; //can be int as _blockSize is int + int newBlockSize = newSize % _blockSize; // can be int as _blockSize is int ssize_t res = 0; IORequest req; @@ -379,7 +383,7 @@ int BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) { } // 3. only necessary if write is forced and block is non 0 length - if ((res >= 0) && forceWrite && newBlockSize) { + if ((res >= 0) && forceWrite && (newBlockSize != 0)) { req.offset = newLastBlock * _blockSize; req.dataLen = newBlockSize; memset(mb.data, 0, req.dataLen); @@ -401,7 +405,7 @@ int BlockFileIO::padFile(off_t oldSize, off_t newSize, bool forceWrite) { * Returns 0 in case of success, or -errno in case of failure. */ int BlockFileIO::truncateBase(off_t size, FileIO *base) { - int partialBlock = size % _blockSize; //can be int as _blockSize is int + int partialBlock = size % _blockSize; // can be int as _blockSize is int int res = 0; off_t oldSize = getSize(); diff --git a/encfs/BlockNameIO.cpp b/encfs/BlockNameIO.cpp index e6ebeb9..d8c1070 100644 --- a/encfs/BlockNameIO.cpp +++ b/encfs/BlockNameIO.cpp @@ -166,8 +166,9 @@ int BlockNameIO::encodeName(const char *plaintextName, int length, uint64_t *iv, bool ok; ok = _cipher->blockEncode((unsigned char *)encodedName + 2, length + padding, (uint64_t)mac ^ tmpIV, _key); - if (!ok) + if (!ok) { throw Error("block encode failed in filename encode"); + } // convert to base 64 ascii int encodedStreamLen = length + 2 + padding; @@ -225,8 +226,9 @@ int BlockNameIO::decodeName(const char *encodedName, int length, uint64_t *iv, bool ok; ok = _cipher->blockDecode((unsigned char *)tmpBuf + 2, decodedStreamLen, (uint64_t)mac ^ tmpIV, _key); - if (!ok) + if (!ok) { throw Error("block decode failed in filename decode"); + } // find out true string length int padding = (unsigned char)tmpBuf[2 + decodedStreamLen - 1]; diff --git a/encfs/CipherFileIO.cpp b/encfs/CipherFileIO.cpp index 824356e..ad5c42c 100644 --- a/encfs/CipherFileIO.cpp +++ b/encfs/CipherFileIO.cpp @@ -106,10 +106,9 @@ bool CipherFileIO::setIV(uint64_t iv) { // duh -- there are no file headers for directories! externalIV = iv; return base->setIV(iv); - } else { - VLOG(1) << "setIV failed to re-open for write"; - return false; } + VLOG(1) << "setIV failed to re-open for write"; + return false; } if (fileIV == 0) { if (initHeader() < 0) { @@ -189,11 +188,11 @@ int CipherFileIO::initHeader() { req.data = buf; req.dataLen = 8; ssize_t readSize = base->read(req); - if(readSize < 0) { + if (readSize < 0) { return readSize; } - if(!cipher->streamDecode(buf, sizeof(buf), externalIV, key)) { + if (!cipher->streamDecode(buf, sizeof(buf), externalIV, key)) { return -EBADMSG; } @@ -224,7 +223,7 @@ int CipherFileIO::initHeader() { } while (fileIV == 0); // don't accept 0 as an option.. if (base->isWritable()) { - if(!cipher->streamEncode(buf, sizeof(buf), externalIV, key)) { + if (!cipher->streamEncode(buf, sizeof(buf), externalIV, key)) { return -EBADMSG; } @@ -257,7 +256,7 @@ bool CipherFileIO::writeHeader() { fileIV >>= 8; } - if(!cipher->streamEncode(buf, sizeof(buf), externalIV, key)) { + if (!cipher->streamEncode(buf, sizeof(buf), externalIV, key)) { return false; } @@ -266,11 +265,7 @@ bool CipherFileIO::writeHeader() { req.data = buf; req.dataLen = 8; - if (base->write(req) < 0) { - return false; - } - - return true; + return (base->write(req) >= 0); } /** @@ -319,8 +314,9 @@ int CipherFileIO::generateReverseHeader(unsigned char *headerBuf) { VLOG(1) << "fileIV=" << fileIV; // Encrypt externally-visible header - if(!cipher->streamEncode(headerBuf, HEADER_SIZE, externalIV, key)) + if (!cipher->streamEncode(headerBuf, HEADER_SIZE, externalIV, key)) { return -EBADMSG; + } return 0; } @@ -352,9 +348,13 @@ ssize_t CipherFileIO::readOneBlock(const IORequest &req) const { if (readSize != bs) { VLOG(1) << "streamRead(data, " << readSize << ", IV)"; - ok = streamRead(tmpReq.data, (int)readSize, blockNum ^ fileIV); //cast works because we work on a block and blocksize fit an int + ok = streamRead(tmpReq.data, (int)readSize, + blockNum ^ fileIV); // cast works because we work on a + // block and blocksize fit an int } else { - ok = blockRead(tmpReq.data, (int)readSize, blockNum ^ fileIV); //cast works because we work on a block and blocksize fit an int + ok = blockRead(tmpReq.data, (int)readSize, + blockNum ^ fileIV); // cast works because we work on a + // block and blocksize fit an int } if (!ok) { @@ -389,9 +389,13 @@ ssize_t CipherFileIO::writeOneBlock(const IORequest &req) { bool ok; if (req.dataLen != bs) { - ok = streamWrite(req.data, (int)req.dataLen, blockNum ^ fileIV); //cast works because we work on a block and blocksize fit an int + ok = streamWrite(req.data, (int)req.dataLen, + blockNum ^ fileIV); // cast works because we work on a + // block and blocksize fit an int } else { - ok = blockWrite(req.data, (int)req.dataLen, blockNum ^ fileIV); //cast works because we work on a block and blocksize fit an int + ok = blockWrite(req.data, (int)req.dataLen, + blockNum ^ fileIV); // cast works because we work on a + // block and blocksize fit an int } ssize_t res = 0; @@ -400,8 +404,9 @@ ssize_t CipherFileIO::writeOneBlock(const IORequest &req) { IORequest tmpReq = req; tmpReq.offset += HEADER_SIZE; res = base->write(tmpReq); - } else + } else { res = base->write(req); + } } else { VLOG(1) << "encodeBlock failed for block " << blockNum << ", size " << req.dataLen; @@ -487,7 +492,7 @@ int CipherFileIO::truncate(off_t size) { if (reopen == 1) { reopen = base->open(lastFlags); if (res < 0) { - res = reopen; + res = reopen; } } return res; @@ -536,7 +541,8 @@ ssize_t CipherFileIO::read(const IORequest &origReq) const { VLOG(1) << "Adding " << headerBytes << " header bytes"; // copy the header bytes into the data - int headerOffset = HEADER_SIZE - headerBytes; //can be int as HEADER_SIZE is int + int headerOffset = + HEADER_SIZE - headerBytes; // can be int as HEADER_SIZE is int memcpy(req.data, &headerBuf[headerOffset], headerBytes); // the read does not want data beyond the header @@ -559,7 +565,8 @@ ssize_t CipherFileIO::read(const IORequest &origReq) const { if (readBytes < 0) { return readBytes; // Return error code } - ssize_t sum = headerBytes + readBytes; //could be size_t, but as we return ssize_t... + ssize_t sum = + headerBytes + readBytes; // could be size_t, but as we return ssize_t... VLOG(1) << "returning sum=" << sum; return sum; } diff --git a/encfs/Context.cpp b/encfs/Context.cpp index bfbcbf3..781661d 100644 --- a/encfs/Context.cpp +++ b/encfs/Context.cpp @@ -111,7 +111,8 @@ void EncFS_Context::renameNode(const char *from, const char *to) { // putNode stores "node" under key "path" in the "openFiles" map. It // increments the reference count if the key already exists. -void EncFS_Context::putNode(const char *path, std::shared_ptr node) { +void EncFS_Context::putNode(const char *path, + const std::shared_ptr &node) { Lock lock(contextMutex); auto &list = openFiles[std::string(path)]; // The length of "list" serves as the reference count. @@ -122,7 +123,7 @@ void EncFS_Context::putNode(const char *path, std::shared_ptr node) { // eraseNode is called by encfs_release in response to the RELEASE // FUSE-command we get from the kernel. void EncFS_Context::eraseNode(const char *path, - std::shared_ptr fnode) { + const std::shared_ptr &fnode) { Lock lock(contextMutex); auto it = openFiles.find(std::string(path)); @@ -151,7 +152,7 @@ void EncFS_Context::eraseNode(const char *path, // nextFuseFh returns the next unused uint64 to serve as the FUSE file // handle for the kernel. -uint64_t EncFS_Context::nextFuseFh(void) { +uint64_t EncFS_Context::nextFuseFh() { // This is thread-safe because currentFuseFh is declared as std::atomic return currentFuseFh++; } diff --git a/encfs/Context.h b/encfs/Context.h index 56b0fc4..d77334f 100644 --- a/encfs/Context.h +++ b/encfs/Context.h @@ -48,9 +48,9 @@ class EncFS_Context { void getAndResetUsageCounter(int *usage, int *openCount); - void putNode(const char *path, std::shared_ptr node); + void putNode(const char *path, const std::shared_ptr &node); - void eraseNode(const char *path, std::shared_ptr fnode); + void eraseNode(const char *path, const std::shared_ptr &fnode); void renameNode(const char *oldName, const char *newName); diff --git a/encfs/DirNode.cpp b/encfs/DirNode.cpp index 17a30f4..8dc3335 100644 --- a/encfs/DirNode.cpp +++ b/encfs/DirNode.cpp @@ -149,7 +149,7 @@ class RenameOp { ~RenameOp(); - operator bool() const { return renameList != nullptr; } + explicit operator bool() const { return renameList != nullptr; } bool apply(); void undo(); diff --git a/encfs/FileNode.cpp b/encfs/FileNode.cpp index de65d89..7486720 100644 --- a/encfs/FileNode.cpp +++ b/encfs/FileNode.cpp @@ -246,9 +246,9 @@ ssize_t FileNode::write(off_t offset, unsigned char *data, size_t size) { Lock _lock(mutex); ssize_t res = io->write(req); - //Of course due to encryption we genrally write more than requested + // Of course due to encryption we genrally write more than requested if (res < 0) { - return res; + return res; } return size; } diff --git a/encfs/MACFileIO.cpp b/encfs/MACFileIO.cpp index cf12560..072a7f7 100644 --- a/encfs/MACFileIO.cpp +++ b/encfs/MACFileIO.cpp @@ -147,7 +147,7 @@ off_t MACFileIO::getSize() const { ssize_t MACFileIO::readOneBlock(const IORequest &req) const { int headerSize = macBytes + randBytes; - int bs = blockSize() + headerSize; //ok, should clearly fit into an int + int bs = blockSize() + headerSize; // ok, should clearly fit into an int MemBlock mb = MemoryPool::allocate(bs); @@ -217,7 +217,7 @@ ssize_t MACFileIO::readOneBlock(const IORequest &req) const { ssize_t MACFileIO::writeOneBlock(const IORequest &req) { int headerSize = macBytes + randBytes; - int bs = blockSize() + headerSize; //ok, should clearly fit into an int + int bs = blockSize() + headerSize; // ok, should clearly fit into an int // we have the unencrypted data, so we need to attach a header to it. MemBlock mb = MemoryPool::allocate(bs); @@ -256,7 +256,7 @@ ssize_t MACFileIO::writeOneBlock(const IORequest &req) { int MACFileIO::truncate(off_t size) { int headerSize = macBytes + randBytes; - int bs = blockSize() + headerSize; //ok, should clearly fit into an int + int bs = blockSize() + headerSize; // ok, should clearly fit into an int int res = BlockFileIO::truncateBase(size, nullptr); diff --git a/encfs/RawFileIO.cpp b/encfs/RawFileIO.cpp index 706574c..1b04ffb 100644 --- a/encfs/RawFileIO.cpp +++ b/encfs/RawFileIO.cpp @@ -206,7 +206,8 @@ ssize_t RawFileIO::read(const IORequest &req) const { if (readSize < 0) { int eno = errno; - errno = 0; //just to be sure error seen in integration tests really comes from the function rc. + errno = 0; // just to be sure error seen in integration tests really comes + // from the function rc. RLOG(WARNING) << "read failed at offset " << req.offset << " for " << req.dataLen << " bytes: " << strerror(eno); return -eno; @@ -225,23 +226,25 @@ ssize_t RawFileIO::write(const IORequest &req) { off_t offset = req.offset; /* - * Let's write while pwrite() writes, to avoid writing only a part of the request, - * whereas it could have been fully written. This to avoid inconsistencies / corruption. + * Let's write while pwrite() writes, to avoid writing only a part of the + * request, + * whereas it could have been fully written. This to avoid inconsistencies / + * corruption. */ // while ((bytes != 0) && retrys > 0) { while (bytes != 0) { ssize_t writeSize = ::pwrite(fd, buf, bytes, offset); - if (writeSize <= 0) { + if (writeSize < 0) { int eno = errno; - errno = 0; //just to be sure error seen in integration tests really comes from the function rc. knownSize = false; RLOG(WARNING) << "write failed at offset " << offset << " for " << bytes << " bytes: " << strerror(eno); - // pwrite is not expected to return 0, so eno should always be set, but we never know... - if (eno) { - return -eno; - } + // pwrite is not expected to return 0, so eno should always be set, but we + // never know... + return -eno; + } + if (writeSize == 0) { return -EIO; } @@ -251,7 +254,8 @@ ssize_t RawFileIO::write(const IORequest &req) { } // if (bytes != 0) { - // RLOG(ERROR) << "Write error: wrote " << req.dataLen - bytes << " bytes of " + // RLOG(ERROR) << "Write error: wrote " << req.dataLen - bytes << " bytes of + // " // << req.dataLen << ", max retries reached"; // knownSize = false; // return (eno) ? -eno : -EIO; diff --git a/encfs/SSL_Cipher.cpp b/encfs/SSL_Cipher.cpp index deb16d5..b530fdc 100644 --- a/encfs/SSL_Cipher.cpp +++ b/encfs/SSL_Cipher.cpp @@ -106,7 +106,7 @@ int BytesToKey(int keyLen, int ivLen, const EVP_MD *md, memcpy(iv, mdBuf + offset, toCopy); iv += toCopy; niv -= toCopy; - offset += toCopy; + // offset += toCopy; } if ((nkey == 0) && (niv == 0)) { break; @@ -170,12 +170,14 @@ static Range CAMELLIABlockRange(64, 4096, 16); static std::shared_ptr NewCAMELLIACipher(const Interface &iface, int keyLen) { - if (keyLen <= 0) keyLen = 192; + if (keyLen <= 0) { + keyLen = 192; + } keyLen = CAMELLIAKeyRange.closest(keyLen); - const EVP_CIPHER *blockCipher = 0; - const EVP_CIPHER *streamCipher = 0; + const EVP_CIPHER *blockCipher = nullptr; + const EVP_CIPHER *streamCipher = nullptr; switch (keyLen) { case 128: @@ -503,7 +505,7 @@ CipherKey SSL_Cipher::newRandomKey() { compute a 64-bit check value for the data using HMAC. */ static uint64_t _checksum_64(SSLKey *key, const unsigned char *data, - int dataLen, uint64_t *chainedIV) { + int dataLen, uint64_t *const chainedIV) { rAssert(dataLen > 0); Lock lock(key->mutex); diff --git a/encfs/XmlReader.cpp b/encfs/XmlReader.cpp index 1e4cd20..b28c264 100644 --- a/encfs/XmlReader.cpp +++ b/encfs/XmlReader.cpp @@ -103,7 +103,7 @@ bool XmlValue::readB64(const char *path, unsigned char *data, std::string s = value->text(); s.erase(std::remove_if(s.begin(), s.end(), ::isspace), s.end()); - s.erase(s.find_last_not_of("=") + 1); + s.erase(s.find_last_not_of('=') + 1); int decodedSize = B64ToB256Bytes(s.size()); if (decodedSize != length) { diff --git a/encfs/encfs.cpp b/encfs/encfs.cpp index 1ddbf46..721c28a 100644 --- a/encfs/encfs.cpp +++ b/encfs/encfs.cpp @@ -25,13 +25,13 @@ #include #include #include +#include #include #include #include #include #include #include -#include #ifdef linux #include #endif @@ -65,7 +65,7 @@ using namespace std::placeholders; namespace encfs { -#define GET_FN(ctx, finfo) ctx->getNode((void *)(uintptr_t)finfo->fh) +#define GET_FN(ctx, finfo) (ctx)->getNode((void *)(uintptr_t)(finfo)->fh) static EncFS_Context *context() { return (EncFS_Context *)fuse_get_context()->private_data; @@ -79,9 +79,10 @@ static EncFS_Context *context() { static bool isReadOnly(EncFS_Context *ctx) { return ctx->opts->readOnly; } // helper function -- apply a functor to a cipher path, given the plain path -static int withCipherPath(const char *opName, const char *path, - function op, - bool passReturnCode = false) { +static int withCipherPath( + const char *opName, const char *path, + const function &op, + bool passReturnCode = false) { EncFS_Context *ctx = context(); int res = -EIO; @@ -110,7 +111,7 @@ static int withCipherPath(const char *opName, const char *path, return res; } -static void checkCanary(std::shared_ptr fnode) { +static void checkCanary(const std::shared_ptr &fnode) { if (fnode->canary == CANARY_OK) { return; } @@ -693,7 +694,8 @@ ssize_t _do_read(FileNode *fnode, unsigned char *ptr, size_t size, off_t off) { int encfs_read(const char *path, char *buf, size_t size, off_t offset, struct fuse_file_info *file) { - //Unfortunately we have to convert from ssize_t (pread) to int (fuse), so let's check this will be OK + // Unfortunately we have to convert from ssize_t (pread) to int (fuse), so + // let's check this will be OK if (size > std::numeric_limits::max()) { RLOG(ERROR) << "tried to read too much data: " << size; return -EIO; @@ -714,13 +716,15 @@ int encfs_fsync(const char *path, int dataSync, struct fuse_file_info *file) { return withFileNode("fsync", path, file, bind(_do_fsync, _1, dataSync)); } -ssize_t _do_write(FileNode *fnode, unsigned char *ptr, size_t size, off_t offset) { +ssize_t _do_write(FileNode *fnode, unsigned char *ptr, size_t size, + off_t offset) { return fnode->write(offset, ptr, size); } int encfs_write(const char *path, const char *buf, size_t size, off_t offset, struct fuse_file_info *file) { - //Unfortunately we have to convert from ssize_t (pwrite) to int (fuse), so let's check this will be OK + // Unfortunately we have to convert from ssize_t (pwrite) to int (fuse), so + // let's check this will be OK if (size > std::numeric_limits::max()) { RLOG(ERROR) << "tried to write too much data: " << size; return -EIO;