From 0959e24f86e760192d05986414a83f1768d3798e Mon Sep 17 00:00:00 2001 From: Georges Berenger Date: Fri, 12 Jan 2024 22:35:02 -0800 Subject: [PATCH] Clang-tidy clean-ups (vrs core code) Summary: clang-tidy makes many suggestions. - missing initializations - syntax changes - using modern C++ constructs - static methods when possible - using explict for constructors - "= default" constructors and destructors and other minor things. Differential Revision: D52719364 fbshipit-source-id: e45b5a02c5bfa03e0a7618b7ada117789df2ee67 --- vrs/Compressor.cpp | 6 +-- vrs/ContentBlockReader.cpp | 4 +- vrs/ContentBlockReader.h | 2 +- vrs/DataLayout.cpp | 52 ++++++++----------- vrs/DataLayout.h | 37 +++++++------ vrs/DataLayoutConventions.cpp | 4 +- vrs/DataPieceArray.h | 4 +- vrs/DataPieceString.h | 4 +- vrs/DataPieceStringMap.h | 6 +-- vrs/DataPieceTypes.h | 8 +-- vrs/DataPieceValue.h | 7 +-- vrs/DataPieceVector.h | 8 +-- vrs/DataReference.h | 2 +- vrs/DataSource.h | 4 +- vrs/Decompressor.cpp | 2 +- vrs/Decompressor.h | 2 +- vrs/DiskFile.cpp | 13 ++--- vrs/DiskFile.h | 2 +- vrs/FileCache.h | 2 +- vrs/FileDetailsCache.cpp | 12 ++--- vrs/FileFormat.cpp | 2 +- vrs/FileHandler.h | 2 +- vrs/FileHandlerFactory.cpp | 5 +- vrs/FileSpec.cpp | 45 ++++++++-------- vrs/FileSpec.h | 7 ++- vrs/IndexRecord.cpp | 10 ++-- vrs/IndexRecord.h | 32 ++++++------ vrs/LegacyFormatsProvider.h | 4 +- vrs/MultiRecordFileReader.cpp | 3 +- vrs/MultiRecordFileReader.h | 2 +- vrs/NewChunkHandler.h | 2 +- vrs/ProgressLogger.cpp | 4 +- vrs/ProgressLogger.h | 8 +-- vrs/Record.h | 12 ++--- vrs/RecordFileReader.cpp | 8 +-- vrs/RecordFileReader.h | 11 ++-- vrs/RecordFileWriter.cpp | 31 ++++++----- vrs/RecordFileWriter.h | 12 ++--- vrs/RecordFormat.cpp | 18 +++---- vrs/RecordFormat.h | 26 +++++----- vrs/RecordFormatStreamPlayer.h | 2 +- vrs/RecordManager.cpp | 6 +-- vrs/RecordManager.h | 2 +- vrs/Recordable.cpp | 4 +- vrs/Recordable.h | 6 +-- vrs/TelemetryLogger.cpp | 2 +- vrs/TelemetryLogger.h | 10 ++-- vrs/test/ContentBlockReaderTest.cpp | 2 +- vrs/test/CustomBlockTest.cpp | 48 ++++++++--------- vrs/test/DataLayoutFormatTest.cpp | 8 +-- vrs/test/DataLayoutTest.cpp | 44 ++++++++-------- vrs/test/FileHandlerFactoryTest.cpp | 9 ++-- vrs/test/FrameCompressionTest.cpp | 2 +- vrs/test/MultiRecordFileReaderTest.cpp | 13 +++-- vrs/test/RecordFormatFileTest.cpp | 32 ++++++------ vrs/test/RecordFormatTest.cpp | 10 ++-- vrs/test/RecordTest.cpp | 15 +++--- vrs/test/file_tests/ChunkedFileTest.cpp | 2 +- vrs/test/file_tests/DeviceSimulatorTest.cpp | 6 +-- vrs/test/file_tests/FileCacheTest.cpp | 8 +-- vrs/test/file_tests/RecordableTest.cpp | 26 +++++----- vrs/test/file_tests/SimpleFileHandlerTest.cpp | 30 +++++------ .../file_tests/UserRecordsFileHandlerTest.cpp | 6 +-- vrs/test/helpers/TestProcess.cpp | 9 ++-- vrs/test/helpers/TestProcess.h | 12 ++--- 65 files changed, 370 insertions(+), 369 deletions(-) diff --git a/vrs/Compressor.cpp b/vrs/Compressor.cpp index 7438ff71..263ba5e3 100644 --- a/vrs/Compressor.cpp +++ b/vrs/Compressor.cpp @@ -175,7 +175,7 @@ class Compressor::CompressorImpl { zstdContext_ = ZSTD_createCCtx(); } int compressionLevel = sZstdPresets[zstdPreset]; - size_t zresult; + size_t zresult = 0; IF_ZCOMP_ERROR_LOG_AND_RETURN( ZSTD_CCtx_setParameter(zstdContext_, ZSTD_c_compressionLevel, compressionLevel)); IF_ZCOMP_ERROR_LOG_AND_RETURN(ZSTD_CCtx_setPledgedSrcSize(zstdContext_, dataSize)); @@ -188,7 +188,7 @@ class Compressor::CompressorImpl { uint32_t& inOutCompressedSize, size_t maxCompressedSize, bool endFrame) { - size_t zresult; + size_t zresult = 0; do { IF_ZCOMP_ERROR_LOG_AND_RETURN(ZSTD_compressStream2( zstdContext_, output, input, endFrame ? ZSTD_e_end : ZSTD_e_continue)); @@ -210,7 +210,7 @@ class Compressor::CompressorImpl { } private: - const LZ4F_preferences_t* getLz4Preferences(CompressionPreset lz4Preset) const { + static const LZ4F_preferences_t* getLz4Preferences(CompressionPreset lz4Preset) { static LZ4F_preferences_t sLz4FastPreset; static LZ4F_preferences_t sLz4TightPreset; static once_flag sPresetInitFlag; diff --git a/vrs/ContentBlockReader.cpp b/vrs/ContentBlockReader.cpp index c3a90849..36de0f76 100644 --- a/vrs/ContentBlockReader.cpp +++ b/vrs/ContentBlockReader.cpp @@ -112,7 +112,7 @@ ContentBlockReader::~ContentBlockReader() = default; size_t ContentBlockReader::findContentBlockSize( const CurrentRecord& record, RecordFormatStreamPlayer& player) { - uint32_t size32; + uint32_t size32 = 0; // Have we successfully mapped the content block size already? if (contentBlockSizeSpec_ && contentBlockSizeSpec_->isMapped() && contentBlockSizeSpec_->nextContentBlockSize.get(size32)) { @@ -229,7 +229,7 @@ bool AudioBlockReader::findAudioSpec( bool AudioBlockReader::audioContentFromAudioSpec( const datalayout_conventions::AudioSpec& audioSpec, - ContentBlock& audioContentBlock) const { + ContentBlock& audioContentBlock) { AudioSampleFormat sampleFormat = AudioSampleFormat::UNDEFINED; uint8_t numChannels = 0; uint32_t sampleRate = 0; diff --git a/vrs/ContentBlockReader.h b/vrs/ContentBlockReader.h index 97536934..3ca455c1 100644 --- a/vrs/ContentBlockReader.h +++ b/vrs/ContentBlockReader.h @@ -94,7 +94,7 @@ class AudioBlockReader : public ContentBlockReader { protected: bool readAudioContentBlock(const CurrentRecord&, RecordFormatStreamPlayer&, const ContentBlock&); - bool audioContentFromAudioSpec(const datalayout_conventions::AudioSpec&, ContentBlock&) const; + static bool audioContentFromAudioSpec(const datalayout_conventions::AudioSpec&, ContentBlock&); bool findAudioSpec( const CurrentRecord&, RecordFormatStreamPlayer&, diff --git a/vrs/DataLayout.cpp b/vrs/DataLayout.cpp index efacf35e..9967a05e 100644 --- a/vrs/DataLayout.cpp +++ b/vrs/DataLayout.cpp @@ -599,8 +599,9 @@ DataPieceString* DataLayout::findDataPieceString(const string& label) { const_cast(this)->findDataPieceString(label)); } -void DataLayout::forEachDataPiece(function callback, DataPieceType type) - const { +void DataLayout::forEachDataPiece( + const function& callback, + DataPieceType type) const { if (type == DataPieceType::Undefined || type == DataPieceType::Value || type == DataPieceType::Array) { for (const DataPiece* piece : fixedSizePieces_) { @@ -618,7 +619,7 @@ void DataLayout::forEachDataPiece(function callback, Dat } } -void DataLayout::forEachDataPiece(function callback, DataPieceType type) { +void DataLayout::forEachDataPiece(const function& callback, DataPieceType type) { if (type == DataPieceType::Undefined || type == DataPieceType::Value || type == DataPieceType::Array) { for (DataPiece* piece : fixedSizePieces_) { @@ -795,19 +796,19 @@ void adjustPrecision(const double& v, ostream& str) { // Print char/int8/uint8 as numbers // https://stackoverflow.com/questions/19562103/uint8-t-cant-be-printed-with-cout namespace special_chars { -static ostream& operator<<(ostream& os, char c) { +ostream& operator<<(ostream& os, char c) { return is_signed::value ? os << static_cast(c) : os << static_cast(c); } -static ostream& operator<<(ostream& os, signed char c) { +ostream& operator<<(ostream& os, signed char c) { return os << static_cast(c); } -static ostream& operator<<(ostream& os, unsigned char c) { +ostream& operator<<(ostream& os, unsigned char c) { return os << static_cast(c); } -static ostream& operator<<(ostream& os, const string& s) { +ostream& operator<<(ostream& os, const string& s) { return std::operator<<(os, helpers::make_printable(s)); } @@ -1087,12 +1088,9 @@ bool DataPieceValue::isSame(const DataPiece* rhs) const { if (!DataPiece::isSame(rhs)) { return false; } - const DataPieceValue& other = *reinterpret_cast*>(rhs); - if (!vrs::isSame(this->defaultValue_, other.defaultValue_) || - !vrs::isSame(this->properties_, other.properties_)) { - return false; - } - return true; + const auto* other = reinterpret_cast*>(rhs); + return vrs::isSame(this->defaultValue_, other->defaultValue_) && + vrs::isSame(this->properties_, other->properties_); } template @@ -1196,12 +1194,9 @@ bool DataPieceArray::isSame(const DataPiece* rhs) const { if (!DataPiece::isSame(rhs)) { return false; } - const DataPieceArray& other = *reinterpret_cast*>(rhs); - if (!vrs::isSame(this->defaultValues_, other.defaultValues_) || - !vrs::isSame(this->properties_, other.properties_)) { - return false; - } - return true; + const auto* other = reinterpret_cast*>(rhs); + return vrs::isSame(this->defaultValues_, other->defaultValues_) && + vrs::isSame(this->properties_, other->properties_); } template @@ -1272,9 +1267,9 @@ size_t DataPieceVector::collectVariableData(int8_t* data, size_t bufferS template <> bool DataPieceVector::get(vector& outValues) const { - size_t byteCount; + size_t byteCount = 0; const int8_t* source = layout_.getVarData(offset_, byteCount); - uint32_t vectorSize; + uint32_t vectorSize = 0; size_t readSize = 0; if (loadElement(vectorSize, source, readSize, byteCount)) { if ((vectorSize + 1) * sizeof(uint32_t) <= byteCount) { @@ -1456,7 +1451,7 @@ size_t DataPieceStringMap::collectVariableData(int8_t* data, size_t bufferSiz template bool DataPieceStringMap::get(map& outValues) const { outValues.clear(); - size_t dataSize; + size_t dataSize = 0; const int8_t* ptr = layout_.getVarData(offset_, dataSize); size_t readSize = 0; if (ptr != nullptr && dataSize > 0) { @@ -1530,11 +1525,8 @@ bool DataPieceStringMap::isSame(const DataPiece* rhs) const { if (!DataPiece::isSame(rhs)) { return false; } - const DataPieceStringMap& other = *reinterpret_cast*>(rhs); - if (!vrs::isSame(this->defaultValues_, other.defaultValues_)) { - return false; - } - return true; + const auto* other = reinterpret_cast*>(rhs); + return vrs::isSame(this->defaultValues_, other->defaultValues_); } template @@ -1578,13 +1570,13 @@ size_t DataPieceString::collectVariableData(int8_t* data, size_t bufferSize) con } string DataPieceString::get() const { - size_t size; + size_t size = 0; const char* ptr = layout_.getVarData(offset_, size); return ptr != nullptr ? string(ptr, size) : defaultString_; } bool DataPieceString::get(string& outString) const { - size_t size; + size_t size = 0; const char* ptr = layout_.getVarData(offset_, size); if (ptr != nullptr) { outString.resize(0); @@ -1596,7 +1588,7 @@ bool DataPieceString::get(string& outString) const { } bool DataPieceString::isAvailable() const { - size_t count; + size_t count = 0; return layout_.getVarData(offset_, count) != nullptr; } diff --git a/vrs/DataLayout.h b/vrs/DataLayout.h index ac562d4d..00283179 100644 --- a/vrs/DataLayout.h +++ b/vrs/DataLayout.h @@ -92,8 +92,8 @@ struct JsonFormatProfileSpec { bool required = true; ///< Include the required flag. // Default format - JsonFormatProfileSpec() {} - JsonFormatProfileSpec(JsonFormatProfile profile); + JsonFormatProfileSpec() = default; + explicit JsonFormatProfileSpec(JsonFormatProfile profile); }; /// \brief The DataLayout class describes the data stored inside a DataLayoutContentBlock. @@ -188,10 +188,11 @@ struct JsonFormatProfileSpec { class DataLayout { protected: DataLayout() = default; + + public: DataLayout& operator=(const DataLayout&) = delete; DataLayout(const DataLayout&) = delete; - public: /// DataLayout has no virtual method, but it is used in containers, and some of its /// derived classes have important clean-up work to do in their destructor. /// Therefore, DataLayout requires a virtual destructor. @@ -395,11 +396,11 @@ class DataLayout { /// @param callback: a function to call for each element found. /// @param type: filter to select only an element type, of UNDEFINED for no filtering. void forEachDataPiece( - std::function, + const std::function&, DataPieceType type = DataPieceType::Undefined) const; /// Same as above, but as a non-const version. void forEachDataPiece( - std::function, + const std::function&, DataPieceType type = DataPieceType::Undefined); /// For debugging: validate that the index for the variable size data looks valid. @@ -501,7 +502,7 @@ class DataLayout { /// Buffer to hold fixed-size pieces, and the index of var size pieces (if any). vector fixedData_; /// Byte count for all the fixed size pieces + var size index. - size_t fixedDataSizeNeeded_; + size_t fixedDataSizeNeeded_{}; /// Buffer holding variable-size pieces, after they've been collected, or read from disk. vector varData_; /// Tells all the required pieces have been mapped successfully. @@ -561,7 +562,7 @@ class ManualDataLayout : public DataLayout { ManualDataLayout(); /// For manual construction, based on an existing layout, cloning all the pieces. /// Add more pieces using "add()", but don't forget to call endLayout() when you're done. - ManualDataLayout(const DataLayout& layout); + explicit ManualDataLayout(const DataLayout& layout); ~ManualDataLayout() override; @@ -613,19 +614,21 @@ class ManualDataLayout : public DataLayout { /// Use the alternate macro DATA_LAYOUT_STRUCT_WITH_INIT if you need your DataLayoutStruct to be /// initialized by an init() method, to assign default values to DataPiece fields for instance. struct DataLayoutStruct { - DataLayoutStruct(const string& structName); - void dataLayoutStructEnd(const string& structName); + explicit DataLayoutStruct(const string& structName); + static void dataLayoutStructEnd(const string& structName); }; -#define DATA_LAYOUT_STRUCT(DATA_LAYOUT_STRUCT_TYPE) \ - DATA_LAYOUT_STRUCT_TYPE(const std::string& _structName_) : DataLayoutStruct(_structName_) { \ - dataLayoutStructEnd(_structName_); \ +#define DATA_LAYOUT_STRUCT(DATA_LAYOUT_STRUCT_TYPE) \ + explicit DATA_LAYOUT_STRUCT_TYPE(const std::string& _structName_) \ + : DataLayoutStruct(_structName_) { \ + dataLayoutStructEnd(_structName_); \ } -#define DATA_LAYOUT_STRUCT_WITH_INIT(DATA_LAYOUT_STRUCT_TYPE) \ - DATA_LAYOUT_STRUCT_TYPE(const std::string& _structName_) : DataLayoutStruct(_structName_) { \ - dataLayoutStructEnd(_structName_); \ - init(); \ +#define DATA_LAYOUT_STRUCT_WITH_INIT(DATA_LAYOUT_STRUCT_TYPE) \ + explicit DATA_LAYOUT_STRUCT_TYPE(const std::string& _structName_) \ + : DataLayoutStruct(_structName_) { \ + dataLayoutStructEnd(_structName_); \ + init(); \ } /// \brief Helper class to include DataLayout structs containing a sliced array of DataPieceXXX and @@ -685,7 +688,7 @@ struct DataLayoutStructArray : public vrs::DataLayoutStruct { template class OptionalDataPieces : public std::unique_ptr { public: - OptionalDataPieces(bool allocateFields) + explicit OptionalDataPieces(bool allocateFields) : std::unique_ptr( allocateFields ? std::make_unique() : nullptr) {} }; diff --git a/vrs/DataLayoutConventions.cpp b/vrs/DataLayoutConventions.cpp index a06c5a4b..c0d409a2 100644 --- a/vrs/DataLayoutConventions.cpp +++ b/vrs/DataLayoutConventions.cpp @@ -60,14 +60,14 @@ ContentBlock ImageSpec::getImageContentBlock(const ImageContentBlockSpec& base, uint32_t readStride = stride.get(); // get value or 0 uint32_t readStride2 = stride2.get(); // get value or 0 if (base.getImageFormat() == ImageFormat::RAW) { - return ContentBlock(readPixelFormat, readWidth, readHeight, readStride, readStride2); + return {readPixelFormat, readWidth, readHeight, readStride, readStride2}; } else if (base.getImageFormat() == ImageFormat::VIDEO) { if (blockSize != ContentBlock::kSizeUnknown) { string aCodecName; if (!codecName.get(aCodecName) || aCodecName.empty()) { aCodecName = base.getCodecName(); } - uint32_t aCodecQuality; + uint32_t aCodecQuality = 0; if (!codecQuality.get(aCodecQuality) || !ImageContentBlockSpec::isQualityValid(aCodecQuality)) { aCodecQuality = base.getCodecQuality(); diff --git a/vrs/DataPieceArray.h b/vrs/DataPieceArray.h index 1173ced8..a4c32b6e 100644 --- a/vrs/DataPieceArray.h +++ b/vrs/DataPieceArray.h @@ -52,7 +52,7 @@ class DataPieceArray : public DataPiece { } /// @param bundle: Bundle to reconstruct a DataPieceArray from disk. /// @internal - DataPieceArray(const MakerBundle& bundle); + explicit DataPieceArray(const MakerBundle& bundle); /// Get the size of the array. size_t getArraySize() const { @@ -327,7 +327,7 @@ class DataPieceArray : public DataPiece { } private: - const size_t count_; + const size_t count_{}; map properties_; vector defaultValues_; }; diff --git a/vrs/DataPieceString.h b/vrs/DataPieceString.h index 4ca2f0b2..693897da 100644 --- a/vrs/DataPieceString.h +++ b/vrs/DataPieceString.h @@ -36,11 +36,11 @@ using std::string; class DataPieceString : public DataPiece { public: /// @param label: Name for the DataPiece. - DataPieceString(const string& label) + explicit DataPieceString(const string& label) : DataPiece(label, DataPieceType::String, DataLayout::kVariableSize) {} /// @param bundle: Bundle to reconstruct a DataPieceString from disk. /// @internal - DataPieceString(const MakerBundle& bundle); + explicit DataPieceString(const MakerBundle& bundle); /// Get the DataPiece element type name. /// @return "string". diff --git a/vrs/DataPieceStringMap.h b/vrs/DataPieceStringMap.h index 503af7a1..dcdbec8b 100644 --- a/vrs/DataPieceStringMap.h +++ b/vrs/DataPieceStringMap.h @@ -39,11 +39,11 @@ template class DataPieceStringMap : public DataPiece { public: /// @param label: Name for the DataPiece. - DataPieceStringMap(const string& label) + explicit DataPieceStringMap(const string& label) : DataPiece(label, DataPieceType::StringMap, DataLayout::kVariableSize) {} /// @param bundle: Bundle to reconstruct a DataPieceStringMap from disk. /// @internal - DataPieceStringMap(const MakerBundle& bundle); + explicit DataPieceStringMap(const MakerBundle& bundle); /// Get the name of the element type . /// @internal @@ -104,7 +104,7 @@ class DataPieceStringMap : public DataPiece { /// Tell if the DataPiece is available, directly or mapped successfully. /// @return True if values can be read without using default values. bool isAvailable() const override { - size_t count; + size_t count = 0; return layout_.getVarData(offset_, count) != nullptr; } diff --git a/vrs/DataPieceTypes.h b/vrs/DataPieceTypes.h index 2b1d413a..f70f9f80 100644 --- a/vrs/DataPieceTypes.h +++ b/vrs/DataPieceTypes.h @@ -103,10 +103,11 @@ struct PointND { bool operator!=(const PointND& rhs) const { return !operator==(rhs); } - void operator=(const T rhs[N]) { + PointND& operator=(const T rhs[N]) { for (size_t s = 0; s < N; s++) { dim[s] = rhs[s]; } + return *this; } // Convenience aliases for the dimensions that actually exist @@ -176,7 +177,7 @@ struct MatrixND { using type = T; static constexpr size_t kMatrixSize = N; - MatrixND() {} + MatrixND() = default; MatrixND(const T arr[N][N]) { operator=(arr); } @@ -199,10 +200,11 @@ struct MatrixND { bool operator!=(const MatrixND& rhs) const { return !operator==(rhs); } - void operator=(const T rhs[N][N]) { + MatrixND& operator=(const T rhs[N][N]) { for (size_t s = 0; s < N; s++) { points[s] = rhs[s]; } + return *this; } PointND points[N]; diff --git a/vrs/DataPieceValue.h b/vrs/DataPieceValue.h index aa731394..4dbbca48 100644 --- a/vrs/DataPieceValue.h +++ b/vrs/DataPieceValue.h @@ -41,7 +41,8 @@ class DataPieceValue : public DataPiece { "DataPieceValue does not support bool. Use vrs::Bool instead"); /// @param label: Name for the DataPiece. - DataPieceValue(const string& label) : DataPiece(label, DataPieceType::Value, sizeof(T)) {} + explicit DataPieceValue(const string& label) + : DataPiece(label, DataPieceType::Value, sizeof(T)) {} /// @param label: Name for the DataPiece. /// @param defaultValue: Default value for the DataPiece. DataPieceValue(const string& label, T defaultValue) @@ -50,7 +51,7 @@ class DataPieceValue : public DataPiece { } /// @param bundle: Bundle to reconstruct a DataPieceValue from disk. /// @internal - DataPieceValue(const MakerBundle& bundle); + explicit DataPieceValue(const MakerBundle& bundle); /// Get the name of the element type . /// @internal @@ -285,7 +286,7 @@ template class DataPieceEnum : public DataPieceValue { public: /// @param label: Name for the DataPiece. - DataPieceEnum(const string& label) : DataPieceValue(label) {} + explicit DataPieceEnum(const string& label) : DataPieceValue(label) {} /// @param label: Name for the DataPiece. /// @param defaultValue: Default value for the DataPiece. diff --git a/vrs/DataPieceVector.h b/vrs/DataPieceVector.h index bd661387..15833c86 100644 --- a/vrs/DataPieceVector.h +++ b/vrs/DataPieceVector.h @@ -37,11 +37,11 @@ template class DataPieceVector : public DataPiece { public: /// @param label: Name for the DataPiece. - DataPieceVector(const string& label) + explicit DataPieceVector(const string& label) : DataPiece(label, DataPieceType::Vector, DataLayout::kVariableSize) {} /// @param bundle: Bundle to reconstruct a DataPieceVector from disk. /// @internal - DataPieceVector(const MakerBundle& bundle); + explicit DataPieceVector(const MakerBundle& bundle); /// Get the name of the element type . /// @internal @@ -106,7 +106,7 @@ class DataPieceVector : public DataPiece { /// @param outValues: Reference to a vector for the read or default values. /// @return True if outValues was set from read values (maybe mapped), not default values. bool get(vector& outValues) const { - size_t count; + size_t count = 0; const T* ptr = layout_.getVarData(offset_, count); if (ptr != nullptr && count > 0) { outValues.resize(count); @@ -151,7 +151,7 @@ class DataPieceVector : public DataPiece { /// Tell if the DataPiece is available, directly or mapped successfully. /// @return True if values can be read without using default values. bool isAvailable() const override { - size_t count; + size_t count = 0; return layout_.getVarData(offset_, count) != nullptr; } diff --git a/vrs/DataReference.h b/vrs/DataReference.h index 6f649fa6..4483b1eb 100644 --- a/vrs/DataReference.h +++ b/vrs/DataReference.h @@ -62,7 +62,7 @@ class DataReference { /// @param size1: Size of first block of bytes. /// @param data2: Pointer to second block of bytes. /// @param size2: Size of second block of bytes. - DataReference( + explicit DataReference( void* data1 = nullptr, uint32_t size1 = 0, void* data2 = nullptr, diff --git a/vrs/DataSource.h b/vrs/DataSource.h index 95108576..df77a9e1 100644 --- a/vrs/DataSource.h +++ b/vrs/DataSource.h @@ -39,7 +39,7 @@ class DataLayout; /// briefly for the purpose of calling createRecord(). struct DataLayoutChunk { /// Default constructor to no DataLayout. - DataLayoutChunk() {} + DataLayoutChunk() = default; /// Constructor to reference a DataLayout, which must outlive this DataLayoutChunk. explicit DataLayoutChunk(DataLayout& dataLayout); @@ -80,7 +80,7 @@ class DataSourceChunk { template /* implicit */ DataSourceChunk(const vector& vectorT) : data_{vectorT.data()}, size_{sizeof(T) * vectorT.size()} {} - virtual ~DataSourceChunk() {} + virtual ~DataSourceChunk() = default; /// Constructor for a trivially copyable type T. template < diff --git a/vrs/Decompressor.cpp b/vrs/Decompressor.cpp index c5c51f54..d1e8ea03 100644 --- a/vrs/Decompressor.cpp +++ b/vrs/Decompressor.cpp @@ -205,7 +205,7 @@ int Decompressor::readFrame( void* dst, size_t frameSize, size_t& inOutMaxReadSize) { - size_t zresult; + size_t zresult = 0; IF_DECOMP_ERROR_LOG_AND_RETURN(ZSTD_initDStream(zstdContext_->getContext())); if (getCompressedDataSize() < zresult) { READ_OR_LOG_AND_RETURN(zresult - getCompressedDataSize()); diff --git a/vrs/Decompressor.h b/vrs/Decompressor.h index c8851e04..5074587f 100644 --- a/vrs/Decompressor.h +++ b/vrs/Decompressor.h @@ -102,7 +102,7 @@ class Decompressor { class ZstdDecompressor; std::unique_ptr zstdContext_; std::vector compressedBuffer_; - CompressionType compressionType_; + CompressionType compressionType_{}; size_t readSize_ = {}; size_t decodedSize_ = {}; size_t lastResult_ = {}; diff --git a/vrs/DiskFile.cpp b/vrs/DiskFile.cpp index c217454e..52afa1e5 100644 --- a/vrs/DiskFile.cpp +++ b/vrs/DiskFile.cpp @@ -66,7 +66,7 @@ int DiskFile::close() { lastError_ = 0; for (auto& chunk : chunks_) { if (chunk.file != nullptr) { - int error; + int error = 0; if (!readOnly_) { error = ::fflush(chunk.file); if (error != 0 && lastError_ == 0) { @@ -174,8 +174,9 @@ int64_t DiskFile::getTotalSize() const { vector> DiskFile::getFileChunks() const { vector> chunks; + chunks.reserve(chunks_.size()); for (const Chunk& chunk : chunks_) { - chunks.emplace_back(make_pair(chunk.path, chunk.size)); + chunks.emplace_back(chunk.path, chunk.size); } return chunks; } @@ -494,7 +495,7 @@ int readZstdFileTemplate(const string& path, T& outContent) { return (fileSize < 0) ? FAILURE : 0; } Decompressor decompressor; - size_t frameSize; + size_t frameSize = 0; size_t maxReadSize = static_cast(fileSize); IF_ERROR_LOG_AND_RETURN(decompressor.initFrame(file, frameSize, maxReadSize)); outContent.resize(frameSize / sizeof(typename T::value_type)); @@ -520,7 +521,7 @@ int DiskFile::readZstdFile(const string& path, void* data, size_t dataSize) { return (fileSize < 0) ? FAILURE : 0; } Decompressor decompressor; - size_t frameSize; + size_t frameSize = 0; size_t maxReadSize = static_cast(fileSize); IF_ERROR_LOG_AND_RETURN(decompressor.initFrame(file, frameSize, maxReadSize)); if (frameSize != dataSize) { @@ -552,7 +553,7 @@ int DiskFile::writeTextFile(const std::string& path, const std::string& text) { return file.close(); } -int DiskFile::parseUri(FileSpec& inOutFileSpec, size_t colonIndex) const { +int DiskFile::parseUri(FileSpec& inOutFileSpec, size_t /*colonIndex*/) const { string scheme; string path; map queryParams; @@ -585,7 +586,7 @@ int AtomicDiskFile::close() { string currentName = chunks_.front().path; IF_ERROR_RETURN(DiskFile::close()); int retry = 3; - int status; + int status = 0; while ((status = os::rename(currentName, finalName_)) != 0 && os::isFile(currentName) && retry-- > 0) { os::remove(finalName_); // if there was a collision, make room diff --git a/vrs/DiskFile.h b/vrs/DiskFile.h index 3eb041a6..3e3279f2 100644 --- a/vrs/DiskFile.h +++ b/vrs/DiskFile.h @@ -156,7 +156,7 @@ class DiskFile : public WriteFileHandler { /// @return A status code, 0 for success. static int writeTextFile(const string& path, const string& text); - virtual int parseUri(FileSpec& intOutFileSpec, size_t colonIndex) const override; + int parseUri(FileSpec& intOutFileSpec, size_t colonIndex) const override; protected: int checkChunks(const vector& chunks); diff --git a/vrs/FileCache.h b/vrs/FileCache.h index 4a68f3e8..fb6fe4d6 100644 --- a/vrs/FileCache.h +++ b/vrs/FileCache.h @@ -69,7 +69,7 @@ class FileCache { int getFile(const string& domain, const string& filename, string& outFilePath); protected: - FileCache(const string& mainFolder) : mainFolder_{mainFolder} {} + explicit FileCache(string mainFolder) : mainFolder_{std::move(mainFolder)} {} private: string mainFolder_; diff --git a/vrs/FileDetailsCache.cpp b/vrs/FileDetailsCache.cpp index efccbd81..2851b2e7 100644 --- a/vrs/FileDetailsCache.cpp +++ b/vrs/FileDetailsCache.cpp @@ -44,7 +44,7 @@ static const uint32_t kOriginalFileFormatVersion = FileFormat::fourCharCode('V', /// \brief Helper class to store stream id on disk. struct DiskStreamId { DiskStreamId() : typeId(static_cast(RecordableTypeId::Undefined)), instanceId(0) {} - DiskStreamId(StreamId id) + explicit DiskStreamId(StreamId id) : typeId(static_cast(id.getTypeId())), instanceId(id.getInstanceId()) {} FileFormat::LittleEndian typeId; @@ -59,14 +59,14 @@ struct DiskStreamId { } StreamId getStreamId() const { - return StreamId(getTypeId(), getInstanceId()); + return {getTypeId(), getInstanceId()}; } }; /// \brief Helper class to store record information on disk. struct DiskRecordInfo { - DiskRecordInfo() {} - DiskRecordInfo(const IndexRecord::RecordInfo& record) + DiskRecordInfo() = default; + explicit DiskRecordInfo(const IndexRecord::RecordInfo& record) : timestamp(record.timestamp), recordOffset(record.fileOffset), streamId(record.streamId), @@ -198,7 +198,7 @@ int readIndexData( size_t indexByteSize = indexSize - sizeof(recordableCount) - sizeof(IndexRecord::DiskStreamId) * recordableCount.get() - sizeof(diskIndexSize); while (outIndex.size() < diskIndexSize.get() && indexByteSize > 0) { - size_t frameSize; + size_t frameSize = 0; IF_ERROR_LOG_AND_RETURN(decompressor.initFrame(file, frameSize, indexByteSize)); if (!XR_VERIFY(frameSize % sizeof(DiskRecordInfo) == 0)) { return FAILURE; @@ -287,7 +287,7 @@ int read( return FAILURE; } IF_ERROR_LOG_AND_RETURN(file.setPos(descriptionOffset)); - uint32_t descriptionSize; + uint32_t descriptionSize = 0; IF_ERROR_LOG_AND_RETURN(DescriptionRecord::readDescriptionRecord( file, fileHeader.recordHeaderSize.get(), descriptionSize, outStreamTags, outFileTags)); if (!XR_VERIFY(descriptionOffset + descriptionSize == indexRecordOffset)) { diff --git a/vrs/FileFormat.cpp b/vrs/FileFormat.cpp index 5204791e..a43d420e 100644 --- a/vrs/FileFormat.cpp +++ b/vrs/FileFormat.cpp @@ -275,7 +275,7 @@ bool printVRSFileInternals(FileHandler& file) { } else if (indexFormatVersion == vrs::IndexRecord::kSplitIndexFormatVersion) { cout << "Split File Head.\n"; int64_t currentPos = file.getPos(); - int64_t chunkStart, chunkSize; + int64_t chunkStart{}, chunkSize{}; if (file.getChunkRange(chunkStart, chunkSize) == 0 && XR_VERIFY(currentPos >= chunkStart && currentPos < chunkStart + chunkSize)) { int64_t nextChunkStart = chunkStart + chunkSize; diff --git a/vrs/FileHandler.h b/vrs/FileHandler.h index e5910f7a..1e7a5acb 100644 --- a/vrs/FileHandler.h +++ b/vrs/FileHandler.h @@ -214,7 +214,7 @@ class FileHandler : public FileDelegator { return false; } - virtual bool setStatsCallback(CacheStatsCallbackFunction /* callback */) { + virtual bool setStatsCallback(const CacheStatsCallbackFunction& /* callback */) { return false; } diff --git a/vrs/FileHandlerFactory.cpp b/vrs/FileHandlerFactory.cpp index 309f6c7d..531f9720 100644 --- a/vrs/FileHandlerFactory.cpp +++ b/vrs/FileHandlerFactory.cpp @@ -169,11 +169,10 @@ FileDelegator* FileHandlerFactory::getExtraDelegator(const FileSpec& fileSpec) { XR_LOGE("No {} delegator named {} was registered.", extraName, extraValue); class FailedDelegator : public FileDelegator { public: - int delegateOpen(const FileSpec& fileSpec, unique_ptr& outNewDelegate) - override { + int delegateOpen(const FileSpec&, unique_ptr&) override { return REQUESTED_DELEGATOR_UNAVAILABLE; } - int parseUri(FileSpec& inOutFileSpec, size_t colonIndex) const override { + int parseUri(FileSpec&, size_t) const override { return REQUESTED_DELEGATOR_UNAVAILABLE; } }; diff --git a/vrs/FileSpec.cpp b/vrs/FileSpec.cpp index 1d4e530d..f5ce8768 100644 --- a/vrs/FileSpec.cpp +++ b/vrs/FileSpec.cpp @@ -82,28 +82,31 @@ int FileSpec::parseUri( return INVALID_URI_FORMAT; } // validate url schema - for (size_t p = 0; p < colon && colon != uri.npos; p++) { + for (size_t p = 0; p < colon && colon != string::npos; p++) { unsigned char c = static_cast(uri[p]); // from https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#Generic_syntax - if (p == 0 && !isalpha(c)) { - XR_LOGE("Schema of URI '{}' should start with a letter", uri); - return INVALID_URI_FORMAT; - } - if (p != 0 && !(isalnum(c) || c == '.' || c == '-' || c == '+' || c == '_')) { - XR_LOGE("Schema contains an invalid character {}: {}", c, uri); - return INVALID_URI_FORMAT; + if (p == 0) { + if (!isalpha(c)) { + XR_LOGE("Schema of URI '{}' should start with a letter", uri); + return INVALID_URI_FORMAT; + } + } else { + if (!isalnum(c) && c != '.' && c != '-' && c != '+' && c != '_') { + XR_LOGE("Schema contains an invalid character {}: {}", c, uri); + return INVALID_URI_FORMAT; + } } } auto query = uri.find('?'); // length of path should be longer than 0 - if (query <= colon + 1 || (query == uri.npos && colon >= uri.size() - 1)) { + if (query <= colon + 1 || (query == string::npos && colon >= uri.size() - 1)) { XR_LOGE("Cannot parse input string '{}'. This is not a URI.", uri); return INVALID_URI_FORMAT; } - if (query != uri.npos) { + if (query != string::npos) { size_t start = query + 1; for (size_t p = start; p < uri.size(); p++) { unsigned char c = static_cast(uri[p]); @@ -118,12 +121,12 @@ int FileSpec::parseUri( } } - if (colon != uri.npos && colon > 0) { + if (colon != string::npos && colon > 0) { outScheme = uri.substr(0, colon); } string path = - (colon != uri.npos) ? uri.substr(colon + 1, query - colon - 1) : uri.substr(0, query); + (colon != string::npos) ? uri.substr(colon + 1, query - colon - 1) : uri.substr(0, query); int status = FileSpec::urldecode(path, outPath); if (status != 0) { XR_LOGE("Path contains invalid character {}", path); @@ -144,7 +147,7 @@ int FileSpec::fromPathJsonUri(const string& pathJsonUri, const string& defaultFi return fromJson(pathJsonUri) ? SUCCESS : FILEPATH_PARSE_ERROR; } auto colon = pathJsonUri.find(':'); - bool isUri = colon != pathJsonUri.npos && colon > 1; + bool isUri = colon != string::npos && colon > 1; for (size_t p = 0; p < colon && isUri; p++) { unsigned char c = static_cast(pathJsonUri[p]); // from https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#Generic_syntax @@ -266,7 +269,7 @@ string FileSpec::getSourceLocation() const { auto colon = uri.find(':'); if (colon != string::npos) { auto end = colon; - unsigned char c; + unsigned char c = 0; do { c = static_cast(uri[++end]); } while (c == '/'); @@ -363,27 +366,27 @@ bool FileSpec::hasExtra(const string& name) const { } int FileSpec::getExtraAsInt(const string& name, int defaultValue) const { - int result; + int result = 0; return helpers::getInt(extras, name, result) ? result : defaultValue; } int64_t FileSpec::getExtraAsInt64(const string& name, int64_t defaultValue) const { - int64_t result; + int64_t result = 0; return helpers::getInt64(extras, name, result) ? result : defaultValue; } uint64_t FileSpec::getExtraAsUInt64(const string& name, uint64_t defaultValue) const { - uint64_t result; + uint64_t result = 0; return helpers::getUInt64(extras, name, result) ? result : defaultValue; } double FileSpec::getExtraAsDouble(const string& name, double defaultValue) const { - double result; + double result = 0; return helpers::getDouble(extras, name, result) ? result : defaultValue; } bool FileSpec::getExtraAsBool(const string& name, bool defaultValue) const { - bool result; + bool result = false; return helpers::getBool(extras, name, result) ? result : defaultValue; } @@ -393,7 +396,7 @@ void FileSpec::unsetExtra(const string& name) { int FileSpec::decodeQuery(const string& query, string& outKey, string& outValue) { auto equal = query.find('='); - if (equal == query.npos) { + if (equal == string::npos) { XR_LOGW("'=' doesn't exist in query: {}", query); return INVALID_URI_FORMAT; } @@ -409,7 +412,7 @@ int FileSpec::decodeQuery(const string& query, string& outKey, string& outValue) } string value = query.substr(equal + 1); - if (value.find('=') != value.npos) { + if (value.find('=') != string::npos) { XR_LOGW("More than one '=' in query: {}", query); return INVALID_URI_FORMAT; } diff --git a/vrs/FileSpec.h b/vrs/FileSpec.h index 25161e51..ebccef8d 100644 --- a/vrs/FileSpec.h +++ b/vrs/FileSpec.h @@ -18,7 +18,6 @@ #include -#include #include #include #include @@ -39,13 +38,13 @@ using std::vector; /// If no file handler name is specified, the object is assumed to a set of local files. /// Additional properties may be specified in the extras field, which has helper methods. struct FileSpec { - FileSpec() {} + FileSpec() = default; FileSpec(const string& filehandler, const vector& chunks) : fileHandlerName{filehandler}, chunks{chunks} {} FileSpec(const string& filehandler, const vector&& chunks) : fileHandlerName{filehandler}, chunks{chunks} {} - FileSpec(const vector& chunks) : chunks{chunks} {} - FileSpec(vector&& chunks) : chunks{std::move(chunks)} {} + explicit FileSpec(const vector& chunks) : chunks{chunks} {} + explicit FileSpec(vector&& chunks) : chunks{std::move(chunks)} {} /// clear all the fields. void clear(); diff --git a/vrs/IndexRecord.cpp b/vrs/IndexRecord.cpp index 70274753..c7b0988d 100644 --- a/vrs/IndexRecord.cpp +++ b/vrs/IndexRecord.cpp @@ -352,7 +352,7 @@ int IndexRecord::Reader::readClassicIndexRecord( const size_t indexSize = indexRecordPayloadSize - preludeSize; if (recordCount > 0) { vector recordStructs(recordCount); - int status; + int status = 0; if (uncompressedSize > 0) { Decompressor decompressor; size_t frameSize = 0; @@ -425,7 +425,7 @@ int IndexRecord::Reader::readSplitIndexRecord( int64_t firstUserRecordOffset = fileHeader_.firstUserRecordOffset.get(); bool noRecords = (firstUserRecordOffset == totalFileSize_); int64_t currentPos = file_.getPos(); - int64_t chunkStart, chunkSize; + int64_t chunkStart{}, chunkSize{}; if (!XR_VERIFY(file_.getChunkRange(chunkStart, chunkSize) == 0) || !XR_VERIFY(chunkSize > 0) || !XR_VERIFY( (currentPos >= chunkStart && currentPos < chunkStart + chunkSize) || @@ -491,10 +491,10 @@ int IndexRecord::Reader::readSplitIndexRecord( } else { size_t decompressedRecords = 0; Decompressor decompressor; - int error; + int error = 0; char* endBuffer = reinterpret_cast(recordStructs.data()) + sizeToRead; while (sizeToRead > 0) { - size_t frameSize; + size_t frameSize = 0; BREAK_ON_ERROR(decompressor.initFrame(file_, frameSize, indexByteSize)); BREAK_ON_FALSE(frameSize <= sizeToRead); BREAK_ON_ERROR( @@ -597,7 +597,7 @@ int IndexRecord::Reader::rebuildIndex(bool writeFixedIndex) { } if (hasSplitHeadChunk_) { // go to the start of the second chunk - int64_t chunkStart, chunkSize; + int64_t chunkStart{}, chunkSize{}; if (!XR_VERIFY(file_.getChunkRange(chunkStart, chunkSize) == 0) || !XR_VERIFY(chunkSize > 0) || !XR_VERIFY(chunkStart == 0)) { return REINDEXING_ERROR; diff --git a/vrs/IndexRecord.h b/vrs/IndexRecord.h index ec575e4d..61e7b539 100644 --- a/vrs/IndexRecord.h +++ b/vrs/IndexRecord.h @@ -52,7 +52,7 @@ enum { /// \brief Helper class to store StreamID objects on disk. struct DiskStreamId { DiskStreamId() : typeId(static_cast(RecordableTypeId::Undefined)), instanceId(0) {} - DiskStreamId(StreamId streamId) + explicit DiskStreamId(StreamId streamId) : typeId(static_cast(streamId.getTypeId())), instanceId(streamId.getInstanceId()) {} FileFormat::LittleEndian typeId; @@ -67,13 +67,13 @@ struct DiskStreamId { } StreamId getStreamId() const { - return StreamId(getTypeId(), getInstanceId()); + return {getTypeId(), getInstanceId()}; } }; /// \brief Helper class to store details about a single VRS record on disk. struct DiskRecordInfo { - DiskRecordInfo() {} + DiskRecordInfo() = default; DiskRecordInfo(double timestamp, uint32_t recordSize, StreamId streamId, Record::Type recordType) : timestamp(timestamp), recordSize(recordSize), @@ -103,14 +103,14 @@ struct DiskRecordInfo { /// \brief Helper class to hold the details about a single VRS record in memory. struct RecordInfo { - RecordInfo() {} + RecordInfo() = default; RecordInfo(double timestamp, int64_t fileOffset, StreamId streamId, Record::Type recordType) : timestamp(timestamp), fileOffset(fileOffset), streamId(streamId), recordType(recordType) {} - double timestamp; ///< timestamp of the record - int64_t fileOffset; ///< absolute byte offset of the record in the whole file - StreamId streamId; ///< creator of the record - Record::Type recordType; ///< type of record + double timestamp{}; ///< timestamp of the record + int64_t fileOffset{}; ///< absolute byte offset of the record in the whole file + StreamId streamId{}; ///< creator of the record + Record::Type recordType{}; ///< type of record bool operator<(const RecordInfo& rhs) const { return this->timestamp < rhs.timestamp || @@ -128,7 +128,7 @@ struct RecordInfo { /// \brief Helper class to write VRS index records. class Writer { public: - Writer(FileFormat::FileHeader& fileHeader) + explicit Writer(FileFormat::FileHeader& fileHeader) : fileHeader_{fileHeader}, preallocatedIndexRecordSize_{} {} void reset() { @@ -177,12 +177,12 @@ class Writer { std::unique_ptr splitHeadFile_; // When the file head is split from the user records FileFormat::FileHeader& fileHeader_; FileFormat::RecordHeader splitIndexRecordHeader_; - uint32_t preallocatedIndexRecordSize_; + uint32_t preallocatedIndexRecordSize_{}; Compressor compressor_; set streamIds_; deque writtenRecords_; - size_t writtenBytesCount_; // how many bytes have been written in a partial index - size_t writtenIndexCount_; // how many index entries have been written in the partial index + size_t writtenBytesCount_{}; // how many bytes have been written in a partial index + size_t writtenIndexCount_{}; // how many index entries have been written in the partial index }; /// \brief Helper class to read VRS index records. @@ -228,10 +228,10 @@ class Reader { set& streamIds_; vector& index_; unique_ptr> diskIndex_; // only when rewriting the index - bool indexComplete_; - bool hasSplitHeadChunk_; - int32_t sortErrorCount_; - int32_t droppedRecordCount_; + bool indexComplete_{}; + bool hasSplitHeadChunk_{}; + int32_t sortErrorCount_{}; + int32_t droppedRecordCount_{}; }; /// This is used to count records to different kinds diff --git a/vrs/LegacyFormatsProvider.h b/vrs/LegacyFormatsProvider.h index eaa0c303..4b3d4654 100644 --- a/vrs/LegacyFormatsProvider.h +++ b/vrs/LegacyFormatsProvider.h @@ -34,7 +34,7 @@ class LegacyFormatsProvider; /// Utility class to handle record format registry manipulations class RecordFormatRegistrar { /// Private constructor, since there must be only one instance. - RecordFormatRegistrar() {} + RecordFormatRegistrar() = default; public: // Public interface @@ -118,7 +118,7 @@ class LegacyFormatsProvider { /// block of the RecordFormat, a pointer to a DataLayout must be provided for the matching index. /// @return True if the RecordFormat and the layouts match as expected. Otherwise, false is /// returned and errors will be logged to help debug the problem. - bool addLegacyRecordFormat( + static bool addLegacyRecordFormat( RecordableTypeId typeId, Record::Type recordType, uint32_t formatVersion, diff --git a/vrs/MultiRecordFileReader.cpp b/vrs/MultiRecordFileReader.cpp index 03553e35..7ed4c65d 100644 --- a/vrs/MultiRecordFileReader.cpp +++ b/vrs/MultiRecordFileReader.cpp @@ -740,8 +740,7 @@ const MultiRecordFileReader::StreamIdReaderPair* MultiRecordFileReader::getStrea return &it->second; } -const string& MultiRecordFileReader::getTag(const map& tags, const string& name) - const { +const string& MultiRecordFileReader::getTag(const map& tags, const string& name) { auto iter = tags.find(name); if (iter != tags.end()) { return iter->second; diff --git a/vrs/MultiRecordFileReader.h b/vrs/MultiRecordFileReader.h index 758594fe..f66f1879 100644 --- a/vrs/MultiRecordFileReader.h +++ b/vrs/MultiRecordFileReader.h @@ -471,7 +471,7 @@ class MultiRecordFileReader { const StreamIdReaderPair* getStreamIdReaderPair(UniqueStreamId uniqueStreamId) const; - const string& getTag(const map& tags, const string& name) const; + static const string& getTag(const map& tags, const string& name); /// Returns the RecordFileReader corresponding to the given record. /// nullptr is returned in case the given record doesn't belong to any of the underlying readers. diff --git a/vrs/NewChunkHandler.h b/vrs/NewChunkHandler.h index d7a99bdb..f5927fc8 100644 --- a/vrs/NewChunkHandler.h +++ b/vrs/NewChunkHandler.h @@ -63,7 +63,7 @@ class NewChunkNotifier { private: NewChunkHandler* chunkHandler_; string path_; - size_t index_; + size_t index_{}; }; } // namespace vrs diff --git a/vrs/ProgressLogger.cpp b/vrs/ProgressLogger.cpp index b10b6295..33ae4ba4 100644 --- a/vrs/ProgressLogger.cpp +++ b/vrs/ProgressLogger.cpp @@ -39,7 +39,7 @@ void ProgressLogger::setStepCount(int stepCount) { stepCount_ = stepCount; } -ProgressLogger::~ProgressLogger() {} +ProgressLogger::~ProgressLogger() = default; void ProgressLogger::setDetailedProgress(bool detailedProgress) { detailedProgress_ = detailedProgress; @@ -113,6 +113,6 @@ void ProgressLogger::logError(const string& message) { void ProgressLogger::updateStep(size_t /*progress*/, size_t /*maxProgress*/) {} -SilentLogger::~SilentLogger() {} +SilentLogger::~SilentLogger() = default; } // namespace vrs diff --git a/vrs/ProgressLogger.h b/vrs/ProgressLogger.h index e48e397e..1dee2424 100644 --- a/vrs/ProgressLogger.h +++ b/vrs/ProgressLogger.h @@ -35,7 +35,7 @@ class ProgressLogger { /// silent, unless a slow re-indexing operation is required. /// @param detailedProgress: pass true to log every new step, regardless of timing. /// @param updateDelay: time in seconds between updates. - ProgressLogger(bool detailedProgress = false, double updateDelay = kDefaultUpdateDelay); + explicit ProgressLogger(bool detailedProgress = false, double updateDelay = kDefaultUpdateDelay); virtual ~ProgressLogger(); /// Set the number of steps anticipated, if expecting more than one step. @@ -126,11 +126,11 @@ class ProgressLogger { /// \brief Progress logger to ignore all progress notifications. class SilentLogger : public ProgressLogger { public: - ~SilentLogger(); - virtual bool logProgress(const string&, size_t = 0, size_t = 100, bool = false) { + ~SilentLogger() override; + bool logProgress(const string&, size_t = 0, size_t = 100, bool = false) override { return true; } - virtual bool logStatus(const string&, int = 0) { + bool logStatus(const string&, int = 0) override { return true; } }; diff --git a/vrs/Record.h b/vrs/Record.h index 18785487..ddd7c001 100644 --- a/vrs/Record.h +++ b/vrs/Record.h @@ -152,15 +152,15 @@ class Record final { friend class RecordManager; /// Records are created & deleted exclusively by a Recordable's RecordManager. - Record(RecordManager& recordManager) : recordManager_(recordManager) {} + explicit Record(RecordManager& recordManager) : recordManager_(recordManager) {} ~Record() = default; - double timestamp_; - Type recordType_; - uint32_t formatVersion_; + double timestamp_{}; + Type recordType_{}; + uint32_t formatVersion_{}; std::vector buffer_; - size_t bufferUsedSize_; - uint64_t creationOrder_; + size_t bufferUsedSize_{}; + uint64_t creationOrder_{}; RecordManager& recordManager_; }; diff --git a/vrs/RecordFileReader.cpp b/vrs/RecordFileReader.cpp index 869d6d75..4a2209cb 100644 --- a/vrs/RecordFileReader.cpp +++ b/vrs/RecordFileReader.cpp @@ -187,7 +187,7 @@ bool RecordFileReader::isVrsFile(const string& filePath) { XR_LOGW("Open cancelled"); \ return OPERATION_CANCELLED; \ } \ - error__ = operation__; \ + (error__) = operation__; \ } while (false) int RecordFileReader::doOpenFile( @@ -815,7 +815,7 @@ bool RecordFileReader::readConfigRecords( for (auto configRecord : configRecords) { if (configRecord != nullptr) { foundAtLeastOneStream = true; - int status; + int status = 0; if (streamPlayer == nullptr) { status = readRecord(*configRecord); } else { @@ -885,7 +885,7 @@ unique_ptr RecordFileReader::getDataLayout( return layout; } -const string& RecordFileReader::getTag(const map& tags, const string& name) const { +const string& RecordFileReader::getTag(const map& tags, const string& name) { auto iter = tags.find(name); if (iter != tags.end()) { return iter->second; @@ -1105,7 +1105,7 @@ int RecordFileReader::readRecord( return INVALID_DISK_DATA; } uint32_t dataSize = recordSize - recordHeaderSize_; - uint32_t uncompressedDataSize; + uint32_t uncompressedDataSize = 0; RecordReader* reader = nullptr; CompressionType compressionType = recordHeader.getCompressionType(); switch (compressionType) { diff --git a/vrs/RecordFileReader.h b/vrs/RecordFileReader.h index 675df888..5d749f95 100644 --- a/vrs/RecordFileReader.h +++ b/vrs/RecordFileReader.h @@ -17,7 +17,6 @@ #pragma once #include -#include #include #include #include @@ -491,7 +490,7 @@ class RecordFileReader { } /// Set callback function for cache stats - bool setStatsCallback(FileHandler::CacheStatsCallbackFunction statsCallback) { + bool setStatsCallback(const FileHandler::CacheStatsCallbackFunction& statsCallback) { return file_->setStatsCallback(statsCallback); } @@ -553,7 +552,7 @@ class RecordFileReader { const set& configRecords, StreamPlayer* streamPlayer); - const string& getTag(const map& tags, const string& name) const; ///< private + static const string& getTag(const map& tags, const string& name); ///< private bool mightContainContentTypeInDataRecord(StreamId streamId, ContentType type) const; ///< private // Members to read an open VRS file @@ -579,9 +578,9 @@ class RecordFileReader { // Location of the last record searched for a specific stream & record type // The pair: index of the record for the type (query), index of the record in the stream (result) mutable map, pair> lastRequest_; - int64_t endOfUserRecordsOffset_; - uint32_t recordHeaderSize_; - bool fileHasAnIndex_; + int64_t endOfUserRecordsOffset_{}; + uint32_t recordHeaderSize_{}; + bool fileHasAnIndex_{}; }; /// The method to search the nearest record from the index list. diff --git a/vrs/RecordFileWriter.cpp b/vrs/RecordFileWriter.cpp index 2c65aa12..93261d99 100644 --- a/vrs/RecordFileWriter.cpp +++ b/vrs/RecordFileWriter.cpp @@ -93,8 +93,8 @@ class CompressionWorker { : workQueue_{workQueue}, resultsQueue_{resultsQueue}, threadIndex_{threadIndex}, - initCreatedThreadCallback_{initCreatedThreadCallback}, - thread_(&CompressionWorker::threadActivity, this) {} + initCreatedThreadCallback_{std::move(initCreatedThreadCallback)}, + thread_{&CompressionWorker::threadActivity, this} {} ~CompressionWorker() { thread_.join(); } @@ -103,7 +103,7 @@ class CompressionWorker { void threadActivity() { initCreatedThreadCallback_(thread_, ThreadRole::Compression, threadIndex_); - CompressionJob* job; + CompressionJob* job = nullptr; while (workQueue_.waitForJob(job)) { job->performJob(); resultsQueue_.sendJob(job); @@ -127,7 +127,7 @@ struct CompressionThreadsData { void addThreadUntil( size_t maxThreadPoolSize, - InitCreatedThreadCallback initCreatedThreadCallback) { + const InitCreatedThreadCallback& initCreatedThreadCallback) { if (compressionThreadsPool_.size() < maxThreadPoolSize) { compressionThreadsPool_.reserve(maxThreadPoolSize); CompressionWorker* worker = new CompressionWorker( @@ -177,7 +177,7 @@ struct WriterThreadData { atomic hasRecordsReadyToWrite; function maxTimestampProvider; // for auto-writing records atomic autoCollectDelay; // for auto-writing records - double nextAutoCollectTime; // for auto-writing records + double nextAutoCollectTime{}; // for auto-writing records CompressionThreadsData compressionThreadsData_; @@ -196,7 +196,10 @@ struct WriterThreadData { }; struct PurgeThreadData { - PurgeThreadData(function maxTimestampProvider, double autoPurgeDelay, bool purgePaused) + PurgeThreadData( + const function& maxTimestampProvider, + double autoPurgeDelay, + bool purgePaused) : shouldEndThread{false}, maxTimestampProvider{maxTimestampProvider}, autoPurgeDelay{autoPurgeDelay}, @@ -322,7 +325,7 @@ void RecordFileWriter::setCompressionThreadPoolSize(size_t size) { } void RecordFileWriter::setInitCreatedThreadCallback( - InitCreatedThreadCallback initCreatedThreadCallback) { + const InitCreatedThreadCallback& initCreatedThreadCallback) { initCreatedThreadCallback_ = initCreatedThreadCallback; } @@ -444,7 +447,7 @@ void RecordFileWriter::backgroundPurgeThreadActivity() { while (!purgeThreadData_->shouldEndThread && (status == os::EventChannel::Status::SUCCESS || status == os::EventChannel::Status::TIMEOUT)) { - double waitDelay; + double waitDelay = 0; if (purgeThreadData_->purgingPaused_ || purgeThreadData_->autoPurgeDelay <= 0) { waitDelay = 1; } else { @@ -550,7 +553,9 @@ int RecordFileWriter::writeRecordsAsync(double maxTimestamp) { return writerThreadData_->fileError; // if there was some error already, say so } -int RecordFileWriter::autoWriteRecordsAsync(function maxTimestampProvider, double delay) { +int RecordFileWriter::autoWriteRecordsAsync( + const function& maxTimestampProvider, + double delay) { if (writerThreadData_ == nullptr || writerThreadData_->shouldEndThread) { return INVALID_REQUEST; } @@ -563,7 +568,9 @@ int RecordFileWriter::autoWriteRecordsAsync(function maxTimestampProvi return 0; } -int RecordFileWriter::autoPurgeRecords(function maxTimestampProvider, double delay) { +int RecordFileWriter::autoPurgeRecords( + const function& maxTimestampProvider, + double delay) { if (purgeThreadData_ != nullptr) { unique_lock guard{purgeThreadData_->mutex}; purgeThreadData_->maxTimestampProvider = maxTimestampProvider; @@ -968,7 +975,7 @@ struct RecordFileWriter_::RecordWriterData { void RecordFileWriter::writeOneRecord( RecordFileWriter_::RecordWriterData& rwd, Record* record, - const StreamId streamId, + StreamId streamId, Compressor& compressor, uint32_t compressedSize) { double timestamp = record->getTimestamp(); @@ -1098,7 +1105,7 @@ int RecordFileWriter::writeRecordsMultiThread( waitTime = 0; } // Check if we have a results to process - CompressionJob* job; + CompressionJob* job = nullptr; while (compressionThreadsData.resultsQueue.waitForJob(job, waitTime)) { compressionResults.emplace(job->getSortRecord(), job); waitTime = 0; diff --git a/vrs/RecordFileWriter.h b/vrs/RecordFileWriter.h index e38482ca..075fcb8e 100644 --- a/vrs/RecordFileWriter.h +++ b/vrs/RecordFileWriter.h @@ -118,7 +118,7 @@ class RecordFileWriter { /// This is an optional call, but must be performed before calling createFileAsync(). /// @param initCreatedThreadCallback Callback that returns a reference to the thread /// created, the ThreadRole, and the thread index (only used for Compression threads) - void setInitCreatedThreadCallback(InitCreatedThreadCallback initCreatedThreadCallback); + void setInitCreatedThreadCallback(const InitCreatedThreadCallback& initCreatedThreadCallback); /// Take all the records of all the registered and *active* recordables, /// and write them all to disk. All synchronous, won't return until the file is closed. @@ -190,7 +190,7 @@ class RecordFileWriter { /// to the background thread(s) writing records to disk. All records sent to disk are older. /// @param delay: Number of seconds between automated calls to send records for writing. /// @return A status code: 0 if no error occurred, a file system error code otherwise. - int autoWriteRecordsAsync(function maxTimestampProvider, double delay); + int autoWriteRecordsAsync(const function& maxTimestampProvider, double delay); /// To purge old records automatically, when no file is being written. /// Note: while writing a VRS file asynchronously, record purging will automatically be disabled @@ -199,7 +199,7 @@ class RecordFileWriter { /// purged. /// @param delay: Number of seconds between automated calls to purge records. /// @return A status code: 0 if no error occurred, a file system error code otherwise. - int autoPurgeRecords(function maxTimestampProvider, double delay); + int autoPurgeRecords(const function& maxTimestampProvider, double delay); /// Tell if a disk file is being written. /// @return True is a file is being written by this RecordFileWriter instance. @@ -302,7 +302,7 @@ class RecordFileWriter { void writeOneRecord( RecordFileWriter_::RecordWriterData& rwd, Record* record, - const StreamId id, + StreamId id, Compressor& compressor, uint32_t compressedSize); int completeAndCloseFile(); ///< internal @@ -317,10 +317,10 @@ class RecordFileWriter { protected: /// data members valid while a file is being worked on unique_ptr file_; - uint64_t maxChunkSize_; + uint64_t maxChunkSize_{}; unique_ptr newChunkHandler_; FileFormat::FileHeader fileHeader_; - uint32_t lastRecordSize_; + uint32_t lastRecordSize_{}; bool skipFinalizeIndexRecords_ = false; // for unit testing only! unique_ptr> preliminaryIndex_; IndexRecord::Writer indexRecordWriter_; diff --git a/vrs/RecordFormat.cpp b/vrs/RecordFormat.cpp index 77202c46..7b5d1daf 100644 --- a/vrs/RecordFormat.cpp +++ b/vrs/RecordFormat.cpp @@ -366,9 +366,9 @@ void ImageContentBlockSpec::set(ContentParser& parser) { if (imageFormat_ == ImageFormat::UNDEFINED) { XR_LOGE("Could not parse image format '{}' in '{}'", parser.str, parser.source()); } else { - uint32_t width, height, stride, quality, keyFrameIndex; - double keyFrameTimestamp; - array text; + uint32_t width{}, height{}, stride{}, quality{}, keyFrameIndex{}; + double keyFrameTimestamp = 0; + array text = {}; while (parser.next()) { const char* cstr = parser.str.c_str(); int firstChar = static_cast(*cstr); // could be 0, but that's ok @@ -704,7 +704,7 @@ void AudioContentBlockSpec::set(ContentParser& parser) { if (audioFormat_ == AudioFormat::UNDEFINED) { XR_LOGE("Could not parse audio format '{}' in '{}'", parser.str, parser.source()); } else { - unsigned int tmp; + unsigned int tmp = 0; while (parser.next()) { // peek at first character switch (parser.str[0]) { @@ -930,7 +930,7 @@ ContentBlock::ContentBlock(const string& formatStr) { ContentParser parser(formatStr, '/'); contentType_ = ContentTypeFormatConverter::toEnum(parser.str.c_str()); parser.next(); - uint32_t size; + uint32_t size = 0; if (sscanf(parser.str.c_str(), "size=%u", &size) == 1) { size_ = size; parser.next(); @@ -998,7 +998,7 @@ size_t ContentBlock::getBlockSize() const { } RecordFormat ContentBlock::operator+(const ContentBlock& other) const { - return RecordFormat(*this, other); + return {*this, other}; } const ImageContentBlockSpec& ContentBlock::image() const { @@ -1015,7 +1015,7 @@ void RecordFormat::set(const string& format) { blocks_.clear(); ContentParser parser(format, '+'); do { - blocks_.emplace_back(ContentBlock(parser.str)); // do this at least once to get one block! + blocks_.emplace_back(parser.str); // do this at least once to get one block! } while (parser.next()); } @@ -1200,8 +1200,8 @@ void RecordFormat::getRecordFormats( const map& recordFormatRegister, RecordFormatMap& outFormats) { for (const auto& tag : recordFormatRegister) { - Record::Type recordType; - uint32_t formatVersion; + Record::Type recordType{}; + uint32_t formatVersion = 0; if (RecordFormat::parseRecordFormatTagName(tag.first, recordType, formatVersion) && outFormats.find({recordType, formatVersion}) == outFormats.end()) { outFormats[{recordType, formatVersion}].set(tag.second); diff --git a/vrs/RecordFormat.h b/vrs/RecordFormat.h index 5c06902a..664d0c71 100644 --- a/vrs/RecordFormat.h +++ b/vrs/RecordFormat.h @@ -141,7 +141,7 @@ class ImageContentBlockSpec { static constexpr uint8_t kQualityUndefined = 255; static constexpr double kInvalidTimestamp = -1E-308; // arbitrary unrealistic value - ImageContentBlockSpec() {} + ImageContentBlockSpec() = default; ImageContentBlockSpec(const ImageContentBlockSpec&) = default; ImageContentBlockSpec( @@ -309,7 +309,7 @@ class ImageContentBlockSpec { static size_t getBytesPerPixel(PixelFormat pixel); /// Get pixel format presented as a readable string, from which it can be reconstructed. - inline string getPixelFormatAsString(PixelFormat pixelFormat) { + static string getPixelFormatAsString(PixelFormat pixelFormat) { return toString(pixelFormat); } @@ -337,7 +337,7 @@ class ImageContentBlockSpec { /// Specification of an audio content block. class AudioContentBlockSpec { public: - AudioContentBlockSpec() {} + AudioContentBlockSpec() = default; /// Default copy constructor AudioContentBlockSpec(const AudioContentBlockSpec&) = default; /// For audio formats with encoding (mp3, flac, etc). @@ -448,17 +448,17 @@ class AudioContentBlockSpec { /// Get the number of bits per audio sample for a specific audio sample format. static uint8_t getBitsPerSample(AudioSampleFormat sampleFormat); /// Get an audio sample format as a string. - inline string getSampleFormatAsString(AudioSampleFormat sampleFormat) { + static string getSampleFormatAsString(AudioSampleFormat sampleFormat) { return toString(sampleFormat); } private: - AudioFormat audioFormat_; - AudioSampleFormat sampleFormat_; - uint8_t sampleBlockStride_; - uint8_t channelCount_; - uint32_t sampleRate_; - uint32_t sampleCount_; + AudioFormat audioFormat_{}; + AudioSampleFormat sampleFormat_{}; + uint8_t sampleBlockStride_{}; + uint8_t channelCount_{}; + uint32_t sampleRate_{}; + uint32_t sampleCount_{}; }; /// \brief Specification of a VRS record content block. @@ -586,7 +586,7 @@ class ContentBlock { }; /// Map a pair of record type/format version to a record format, for a particular stream. -typedef map, RecordFormat> RecordFormatMap; +using RecordFormatMap = map, RecordFormat>; /// \brief Helper to identify a particular content block within a file. /// @@ -643,7 +643,7 @@ class ContentBlockId { class RecordFormat { public: /// Empty record format definition. - RecordFormat() {} + RecordFormat() = default; /// Default copy constructor RecordFormat(const RecordFormat&) = default; @@ -682,7 +682,7 @@ class RecordFormat { /// @param size: Size of the block, or ContentBlock::kSizeUnknown, if the size of the block isn't /// known/fixed. RecordFormat(ContentType type, size_t size = ContentBlock::kSizeUnknown) { - blocks_.emplace_back(ContentBlock(type, size)); + blocks_.emplace_back(type, size); } /// Append a ContentBlock to this RecordFormat. diff --git a/vrs/RecordFormatStreamPlayer.h b/vrs/RecordFormatStreamPlayer.h index 3b792efc..c674a818 100644 --- a/vrs/RecordFormatStreamPlayer.h +++ b/vrs/RecordFormatStreamPlayer.h @@ -67,7 +67,7 @@ class RecordFormatStreamPlayer : public StreamPlayer { /// Return false, if the record should not be read entirely, for instance, if you only need to /// read some metadata stored in the first content block, but don't need to read & decode the rest /// of the record. - virtual bool onDataLayoutRead(const CurrentRecord& r, size_t /* blockIndex */, DataLayout& dl) { + virtual bool onDataLayoutRead(const CurrentRecord& record, size_t /* blockIndex */, DataLayout&) { return true; // we can go read the next block, if any, since we've read the data } diff --git a/vrs/RecordManager.cpp b/vrs/RecordManager.cpp index 72d64ff7..678950dc 100644 --- a/vrs/RecordManager.cpp +++ b/vrs/RecordManager.cpp @@ -181,7 +181,7 @@ void RecordManager::collectOldRecords(double maxAge, list& outCollected } } -size_t RecordManager::getAcceptableOverCapacity(size_t capacity) const { +size_t RecordManager::getAcceptableOverCapacity(size_t capacity) { return capacity + capacity / 5; // 20% } @@ -194,9 +194,7 @@ void RecordManager::recycle(Record* record) { record = nullptr; } } // mutex scope limiting - if (record != nullptr) { - delete record; - } + delete record; } } // namespace vrs diff --git a/vrs/RecordManager.h b/vrs/RecordManager.h index 58ad7671..35003a73 100644 --- a/vrs/RecordManager.h +++ b/vrs/RecordManager.h @@ -97,7 +97,7 @@ class RecordManager { } private: - size_t getAcceptableOverCapacity(size_t capacity) const; + static size_t getAcceptableOverCapacity(size_t capacity); void recycle(Record* record); std::mutex mutex_; diff --git a/vrs/Recordable.cpp b/vrs/Recordable.cpp index d7dd5840..9b0a17eb 100644 --- a/vrs/Recordable.cpp +++ b/vrs/Recordable.cpp @@ -51,7 +51,7 @@ bool Recordable::addRecordFormat( Record::Type recordType, uint32_t formatVersion, const RecordFormat& format, - vector layouts) { + const vector& layouts) { return RecordFormat::addRecordFormat(tags_.vrs, recordType, formatVersion, format, layouts); } @@ -102,7 +102,7 @@ uint16_t Recordable::getNewInstanceId(RecordableTypeId typeId) { return instanceId; } -const string& Recordable::getTag(const map& tags, const string& name) const { +const string& Recordable::getTag(const map& tags, const string& name) { auto iter = tags.find(name); if (iter != tags.end()) { return iter->second; diff --git a/vrs/Recordable.h b/vrs/Recordable.h index 1cb242ae..a9ed8a0e 100644 --- a/vrs/Recordable.h +++ b/vrs/Recordable.h @@ -57,7 +57,7 @@ class Recordable { /// @param typeId: an id telling which type of stream this is. /// @param flavor: a flavor required when using << recordable class >> type ids. /// Note: you may always provide a flavor, but you are required to with << recordable class> ids. - Recordable(RecordableTypeId typeId, const string& flavor = {}); + explicit Recordable(RecordableTypeId typeId, const string& flavor = {}); public: virtual ~Recordable(); @@ -123,7 +123,7 @@ class Recordable { Record::Type recordType, uint32_t formatVersion, const RecordFormat& format, - vector layouts = {}); + const vector& layouts = {}); /// Configuration records describe how the device recorded is configured/setup. /// The configuration of a recordable is probably not changed by data flowing through. @@ -296,7 +296,7 @@ class Recordable { /// @internal static uint16_t getNewInstanceId(RecordableTypeId typeId = static_cast(0)); - const string& getTag(const map& tags, const string& name) const; + static const string& getTag(const map& tags, const string& name); }; /// Temporarily reset Recordable instance IDs diff --git a/vrs/TelemetryLogger.cpp b/vrs/TelemetryLogger.cpp index 93a30004..38a7480a 100644 --- a/vrs/TelemetryLogger.cpp +++ b/vrs/TelemetryLogger.cpp @@ -78,7 +78,7 @@ unique_ptr& TelemetryLogger::getInstance() { return sInstance; } -TelemetryLogger::~TelemetryLogger() {} +TelemetryLogger::~TelemetryLogger() = default; TrafficEvent& TrafficEvent::setAttemptStartTime() { transferStartTime = os::getCurrentTimeSecSinceEpoch(); diff --git a/vrs/TelemetryLogger.h b/vrs/TelemetryLogger.h index 410a4058..e7a9b550 100644 --- a/vrs/TelemetryLogger.h +++ b/vrs/TelemetryLogger.h @@ -29,19 +29,19 @@ struct OperationContext { string operation; string sourceLocation; - OperationContext() {} + OperationContext() = default; OperationContext(const string& op, const string& sourceLoc) : operation{op}, sourceLocation{sourceLoc} {} OperationContext(const OperationContext& rhs) : operation{rhs.operation}, sourceLocation{rhs.sourceLocation} {} - OperationContext(OperationContext&& rhs) + OperationContext(OperationContext&& rhs) noexcept : operation{std::move(rhs.operation)}, sourceLocation{std::move(rhs.sourceLocation)} {} bool operator<(const OperationContext& rhs) const { auto tie = [](const OperationContext& oc) { return std::tie(oc.operation, oc.sourceLocation); }; return tie(*this) < tie(rhs); } - OperationContext& operator=(OperationContext&& rhs) { + OperationContext& operator=(OperationContext&& rhs) noexcept { operation = std::move(rhs.operation); sourceLocation = std::move(rhs.sourceLocation); return *this; @@ -57,13 +57,13 @@ struct LogEvent { const std::string& message, const string& serverReply) : type{type}, operationContext{opContext}, message{message}, serverReply{serverReply} {} - LogEvent(LogEvent&& rhs) + LogEvent(LogEvent&& rhs) noexcept : type{std::move(rhs.type)}, operationContext{std::move(rhs.operationContext)}, message{std::move(rhs.message)}, serverReply{std::move(rhs.serverReply)} {} - LogEvent& operator=(LogEvent&& rhs) { + LogEvent& operator=(LogEvent&& rhs) noexcept { type = std::move(rhs.type); operationContext = std::move(rhs.operationContext); message = std::move(rhs.message); diff --git a/vrs/test/ContentBlockReaderTest.cpp b/vrs/test/ContentBlockReaderTest.cpp index 5fd8709c..eeee9422 100644 --- a/vrs/test/ContentBlockReaderTest.cpp +++ b/vrs/test/ContentBlockReaderTest.cpp @@ -38,7 +38,7 @@ ContentBlock getImageContentBlock( } bool isImageSpec( - ImageContentBlockSpec spec, + const ImageContentBlockSpec& spec, DataLayout& layout, const ImageContentBlockSpec& base, size_t blockSize = ContentBlock::kSizeUnknown) { diff --git a/vrs/test/CustomBlockTest.cpp b/vrs/test/CustomBlockTest.cpp index 6f8605ea..89e23b7c 100644 --- a/vrs/test/CustomBlockTest.cpp +++ b/vrs/test/CustomBlockTest.cpp @@ -29,30 +29,30 @@ using namespace vrs; namespace { -static const uint32_t kConfigurationVersion = 1; -static const uint32_t kDataVersion = 1; -static const uint32_t kStateVersion = 1; +const uint32_t kConfigurationVersion = 1; +const uint32_t kDataVersion = 1; +const uint32_t kStateVersion = 1; -static const uint32_t kFrameWidth = 640; -static const uint32_t kFrameHeight = 480; -static const uint32_t kPixelByteSize = 1; +const uint32_t kFrameWidth = 640; +const uint32_t kFrameHeight = 480; +const uint32_t kPixelByteSize = 1; -static const size_t kConfigCustomBlockSize0 = 190; -static const size_t kStateCustomBlockSize1 = 25; -static const size_t kStateCustomBlockSize3 = 39; -static const size_t kDataCustomBlockSize1 = 37; -static const size_t kDataCustomBlockSize3 = 125; -static const size_t kDataCustomBlockSize4 = 9; +const size_t kConfigCustomBlockSize0 = 190; +const size_t kStateCustomBlockSize1 = 25; +const size_t kStateCustomBlockSize3 = 39; +const size_t kDataCustomBlockSize1 = 37; +const size_t kDataCustomBlockSize3 = 125; +const size_t kDataCustomBlockSize4 = 9; -static const double kStartTimestamp = 1543864285; +const double kStartTimestamp = 1543864285; -static const size_t kRecordSetCount = 3; +const size_t kRecordSetCount = 3; using namespace datalayout_conventions; // Generate/check a custom block of data with a (very) pseudo random pattern struct CustomBlob { - CustomBlob(size_t size) { + explicit CustomBlob(size_t size) { blob.resize(size); for (size_t k = 0; k < size; k++) { blob[k] = dataAt(k, size); @@ -119,7 +119,7 @@ class DataRecordDataSource : public DataSource { const DataSourceChunk& cb2, const DataSourceChunk& cb3) : DataSource(dl1, dl2, cb1, cb2, cb3) {} - virtual void copyTo(uint8_t* buffer) const { + void copyTo(uint8_t* buffer) const override { dataLayout1_.fillAndAdvanceBuffer(buffer); if (chunk1_.size() > 0) { chunk1_.fillAndAdvanceBuffer(buffer); @@ -220,7 +220,7 @@ class CustomStreams : public Recordable { class CustomStreamPlayer : public RecordFormatStreamPlayer { public: - virtual bool onDataLayoutRead(const CurrentRecord& record, size_t blockIndex, DataLayout& dl) { + bool onDataLayoutRead(const CurrentRecord& record, size_t blockIndex, DataLayout& dl) override { if (record.recordType == Record::Type::CONFIGURATION) { GTEST_NONFATAL_FAILURE_("No datalayout expected for config records"); return false; @@ -234,8 +234,8 @@ class CustomStreamPlayer : public RecordFormatStreamPlayer { } return true; } - virtual bool - onImageRead(const CurrentRecord& record, size_t blockIndex, const ContentBlock& contentBlock) { + bool onImageRead(const CurrentRecord& record, size_t blockIndex, const ContentBlock& contentBlock) + override { EXPECT_EQ(record.recordType, Record::Type::STATE); EXPECT_EQ(blockIndex, 2); size_t size = contentBlock.getBlockSize(); @@ -249,10 +249,10 @@ class CustomStreamPlayer : public RecordFormatStreamPlayer { stateImageCount++; return true; } - virtual bool onCustomBlockRead( + bool onCustomBlockRead( const CurrentRecord& record, size_t blockIndex, - const ContentBlock& contentBlock) { + const ContentBlock& contentBlock) override { size_t size = contentBlock.getBlockSize(); if (size == ContentBlock::kSizeUnknown) { GTEST_NONFATAL_FAILURE_("Unknown custom block size!"); @@ -303,7 +303,7 @@ class CustomStreamPlayer : public RecordFormatStreamPlayer { }; struct CustomBlockTest : testing::Test { - int createFileAtOnce(const string& filePath) { + static int createFileAtOnce(const string& filePath) { RecordFileWriter fileWriter; fileWriter.setTag("purpose", "this is a test"); CustomStreams imageStream; @@ -317,7 +317,7 @@ struct CustomBlockTest : testing::Test { return 0; } - void checkFileHandler(const string& filePath) { + static void checkFileHandler(const string& filePath) { // Verify that the file was created, and looks like we think it should RecordFileReader reader; int openFileStatus = reader.openFile(filePath); @@ -360,7 +360,7 @@ TEST_F(CustomBlockTest, simpleCreation) { } TEST_F(CustomBlockTest, DataSourceChunkTest) { - int anint; + int anint = 0; DataSourceChunk intdl(anint); EXPECT_EQ(intdl.size(), sizeof(int)); diff --git a/vrs/test/DataLayoutFormatTest.cpp b/vrs/test/DataLayoutFormatTest.cpp index 479591db..cb569209 100644 --- a/vrs/test/DataLayoutFormatTest.cpp +++ b/vrs/test/DataLayoutFormatTest.cpp @@ -101,9 +101,9 @@ struct FormatValues : public AutoDataLayout { namespace { -static const ImageSpecType kWidth = 640; -static const ImageSpecType kHeight = 480; -static const ImageSpecType kBytesPerPixel = 1; +const ImageSpecType kWidth = 640; +const ImageSpecType kHeight = 480; +const ImageSpecType kBytesPerPixel = 1; template void check( @@ -229,7 +229,7 @@ class RecordableDevice : public Recordable { {&data_}); } - double getTimeStamp() { + static double getTimeStamp() { static double sTimeStamp = 0; return sTimeStamp += 1; } diff --git a/vrs/test/DataLayoutTest.cpp b/vrs/test/DataLayoutTest.cpp index 653d0ae2..c37ef371 100644 --- a/vrs/test/DataLayoutTest.cpp +++ b/vrs/test/DataLayoutTest.cpp @@ -238,7 +238,7 @@ TEST_F(DataLayoutTester, testDataLayoutMatcher) { // make a missing required field fail the mapping otherConfig.double_.setRequired(); EXPECT_FALSE(otherConfig.mapLayout(testConfig)); - double v; + double v = 0; EXPECT_FALSE(otherConfig.double_.get(v)); EXPECT_FALSE(otherConfig.double_.getDefault(v)); @@ -276,7 +276,7 @@ TEST_F(DataLayoutTester, testDataLayoutMatcher) { EXPECT_FALSE(newConfig.char_.get(chart)); EXPECT_EQ(chart, 'a'); - int8_t int8; + int8_t int8 = 0; EXPECT_EQ(newConfig.int8_.get(), oldConfig.int8_.get()); EXPECT_TRUE(newConfig.int8_.get(int8)); EXPECT_EQ(int8, oldConfig.int8_.get()); @@ -291,7 +291,7 @@ TEST_F(DataLayoutTester, testDataLayoutMatcher) { EXPECT_FALSE(newConfig.float_.isAvailable()); // check missing data, with no default - float floatt; + float floatt = 0; EXPECT_FALSE(newConfig.float_.getDefault(floatt)); EXPECT_EQ(newConfig.float_.get(), 0); floatt = -1; @@ -300,7 +300,7 @@ TEST_F(DataLayoutTester, testDataLayoutMatcher) { // check missing data, with default EXPECT_FALSE(newConfig.double_.isAvailable()); - double doublet_default; + double doublet_default = 0; EXPECT_TRUE(newConfig.double_.getDefault(doublet_default)); EXPECT_EQ(doublet_default, 3.14); EXPECT_EQ(newConfig.double_.get(), doublet_default); @@ -482,7 +482,7 @@ TEST_F(DataLayoutTester, testVarSizeFields) { EXPECT_EQ(strings.size(), 3); EXPECT_TRUE(strings[0] == "Eline"); EXPECT_TRUE(strings[1] == "Marlene"); - EXPECT_TRUE(strings[2] == ""); + EXPECT_TRUE(strings[2].empty()); EXPECT_FALSE(varSizeLayout.int_.isAvailable()); @@ -508,7 +508,7 @@ struct OptionalFields { }; struct LayoutWithOptionalFields : public AutoDataLayout { - LayoutWithOptionalFields(bool allocateOptionalFields = false) + explicit LayoutWithOptionalFields(bool allocateOptionalFields = false) : optionalFields(allocateOptionalFields) {} DataPieceString normalField{"normal_field"}; @@ -560,7 +560,7 @@ struct Counters { template inline void addTemplatePiece(ManualDataLayout& layout, Counters& c, const T& defaultValue) { string valuePieceName = to_string(++c.pieceCounter) + "_value"; - DataPieceValue* newValue = new DataPieceValue(valuePieceName.c_str(), defaultValue); + DataPieceValue* newValue = new DataPieceValue(valuePieceName, defaultValue); layout.add(unique_ptr(newValue)); newValue->setMin(numeric_limits::lowest()); newValue->setMax(numeric_limits::max()); @@ -572,7 +572,7 @@ inline void addTemplatePiece(ManualDataLayout& layout, Counters& c, const T& def newValue->setTag("units", "metric"); string arrayPieceName = to_string(++c.pieceCounter) + "_array"; - DataPieceArray* newArray = new DataPieceArray(arrayPieceName.c_str(), c.arraySize++); + DataPieceArray* newArray = new DataPieceArray(arrayPieceName, c.arraySize++); layout.add(unique_ptr(newArray)); vector values; for (size_t k = 0; k < newArray->getArraySize(); ++k) { @@ -584,13 +584,13 @@ inline void addTemplatePiece(ManualDataLayout& layout, Counters& c, const T& def newArray->setTag(arrayPieceName, valuePieceName); // make something variable... string vectorPieceName = to_string(++c.pieceCounter) + "_vector"; - DataPieceVector* newVector = new DataPieceVector(vectorPieceName.c_str()); + DataPieceVector* newVector = new DataPieceVector(vectorPieceName); layout.add(unique_ptr(newVector)); newVector->setDefault(values); newArray->setTag(vectorPieceName, arrayPieceName); // make something variable... string stringMapPieceName = to_string(++c.pieceCounter) + "_stringMap"; - DataPieceStringMap* newStringMap = new DataPieceStringMap(stringMapPieceName.c_str()); + DataPieceStringMap* newStringMap = new DataPieceStringMap(stringMapPieceName); layout.add(unique_ptr(newStringMap)); map stringMap; stringMap["lowest"] = numeric_limits::lowest(); @@ -612,13 +612,13 @@ T makePoint(size_t baseValue) { template inline void addTemplatePiecePoint(ManualDataLayout& layout, Counters& c) { string valuePieceName = to_string(++c.pieceCounter) + "_value"; - DataPieceValue* newValue = new DataPieceValue(valuePieceName.c_str()); + DataPieceValue* newValue = new DataPieceValue(valuePieceName); layout.add(unique_ptr(newValue)); newValue->setTag("description", string("this is ") + valuePieceName); newValue->setTag("units", "metric"); string arrayPieceName = to_string(++c.pieceCounter) + "_array"; - DataPieceArray* newArray = new DataPieceArray(arrayPieceName.c_str(), c.arraySize++); + DataPieceArray* newArray = new DataPieceArray(arrayPieceName, c.arraySize++); layout.add(unique_ptr(newArray)); vector values; for (size_t k = 0; k < newArray->getArraySize(); ++k) { @@ -630,13 +630,13 @@ inline void addTemplatePiecePoint(ManualDataLayout& layout, Counters& c) { newArray->setTag(arrayPieceName, valuePieceName); // make something variable... string vectorPieceName = to_string(++c.pieceCounter) + "_vector"; - DataPieceVector* newVector = new DataPieceVector(vectorPieceName.c_str()); + DataPieceVector* newVector = new DataPieceVector(vectorPieceName); layout.add(unique_ptr(newVector)); newVector->setDefault(values); newArray->setTag(vectorPieceName, arrayPieceName); // make something variable... string stringMapPieceName = to_string(++c.pieceCounter) + "_stringMap"; - DataPieceStringMap* newStringMap = new DataPieceStringMap(stringMapPieceName.c_str()); + DataPieceStringMap* newStringMap = new DataPieceStringMap(stringMapPieceName); layout.add(unique_ptr(newStringMap)); map stringMap; stringMap["one"] = makePoint(c.arraySize + 0); @@ -658,13 +658,13 @@ T makeMatrix(size_t baseValue) { template inline void addTemplatePieceMatrix(ManualDataLayout& layout, Counters& c) { string valuePieceName = to_string(++c.pieceCounter) + "_value"; - DataPieceValue* newValue = new DataPieceValue(valuePieceName.c_str()); + DataPieceValue* newValue = new DataPieceValue(valuePieceName); layout.add(unique_ptr(newValue)); newValue->setTag("description", string("this is ") + valuePieceName); newValue->setTag("units", "metric"); string arrayPieceName = to_string(++c.pieceCounter) + "_array"; - DataPieceArray* newArray = new DataPieceArray(arrayPieceName.c_str(), c.arraySize++); + DataPieceArray* newArray = new DataPieceArray(arrayPieceName, c.arraySize++); layout.add(unique_ptr(newArray)); vector values; for (size_t k = 0; k < newArray->getArraySize(); ++k) { @@ -676,13 +676,13 @@ inline void addTemplatePieceMatrix(ManualDataLayout& layout, Counters& c) { newArray->setTag(arrayPieceName, valuePieceName); // make something variable... string vectorPieceName = to_string(++c.pieceCounter) + "_vector"; - DataPieceVector* newVector = new DataPieceVector(vectorPieceName.c_str()); + DataPieceVector* newVector = new DataPieceVector(vectorPieceName); layout.add(unique_ptr(newVector)); newVector->setDefault(values); newArray->setTag(vectorPieceName, arrayPieceName); // make something variable... string stringMapPieceName = to_string(++c.pieceCounter) + "_stringMap"; - DataPieceStringMap* newStringMap = new DataPieceStringMap(stringMapPieceName.c_str()); + DataPieceStringMap* newStringMap = new DataPieceStringMap(stringMapPieceName); layout.add(unique_ptr(newStringMap)); map stringMap; stringMap["one"] = makeMatrix(c.arraySize + 0); @@ -722,7 +722,7 @@ TEST_F(DataLayoutTester, testSerialization) { addTemplatePieceMatrix(manualLayout, counters); DataPieceString* stringPiece = - new DataPieceString((to_string(++counters.pieceCounter) + "_string").c_str()); + new DataPieceString(to_string(++counters.pieceCounter) + "_string"); manualLayout.add(unique_ptr(stringPiece)); stringPiece->setDefault("a default string"); @@ -807,7 +807,7 @@ TEST_F(DataLayoutTester, testMetaData) { const DataPieceValue* intValue = dl->findDataPieceValue("int"); ASSERT_NE(intValue, nullptr); - int32_t v; + int32_t v = 0; EXPECT_TRUE(intValue->getMin(v)); EXPECT_EQ(v, 10); EXPECT_TRUE(intValue->getMax(v)); @@ -824,7 +824,7 @@ TEST_F(DataLayoutTester, testMetaData) { const DataPieceValue* floatValue = dl->findDataPieceValue("float"); ASSERT_NE(floatValue, nullptr); - float f; + float f = 0; EXPECT_TRUE(floatValue->getMin(f)); EXPECT_NEAR(f, -10.f, 0.0001f); EXPECT_TRUE(floatValue->getMax(f)); @@ -945,7 +945,7 @@ struct ALayout : public AutoDataLayout { AutoDataLayoutEnd end; }; -static void checkValues(DataLayout& layout) { +void checkValues(DataLayout& layout) { ALayout alayout; alayout.mapLayout(layout); EXPECT_EQ(alayout.int8_.get(), kInt8); diff --git a/vrs/test/FileHandlerFactoryTest.cpp b/vrs/test/FileHandlerFactoryTest.cpp index a1981714..aa87ae1c 100644 --- a/vrs/test/FileHandlerFactoryTest.cpp +++ b/vrs/test/FileHandlerFactoryTest.cpp @@ -39,18 +39,17 @@ struct FileHandlerFactoryTest : testing::Test { // Fake FileHandler class that just pretends to open any path class FakeHandler : public DiskFile { public: - FakeHandler(const string& name) : fileHandlerName_{name} {} + explicit FakeHandler(string name) : fileHandlerName_{std::move(name)} {} unique_ptr makeNew() const override { return make_unique(fileHandlerName_); } - int open(const string& filePath) override { + int open(const string& /*filePath*/) override { return 0; } - int openSpec(const FileSpec& fileSpec) override { + int openSpec(const FileSpec& /*fileSpec*/) override { return 0; } - virtual int delegateOpen(const FileSpec& fileSpec, unique_ptr& outNewDelegate) - override { + int delegateOpen(const FileSpec& /*fileSpec*/, unique_ptr& outNewDelegate) override { outNewDelegate.reset(); return 0; } diff --git a/vrs/test/FrameCompressionTest.cpp b/vrs/test/FrameCompressionTest.cpp index 4140af47..790786b8 100644 --- a/vrs/test/FrameCompressionTest.cpp +++ b/vrs/test/FrameCompressionTest.cpp @@ -375,7 +375,7 @@ int makeAndWriteFrame( size_t& totalSize) { input.resize(frameSize); randomInit(input, root); - uint32_t writtenSize; + uint32_t writtenSize = 0; EXPECT_ZERO_OR_RETURN(compressor.startFrame(input.size(), preset, writtenSize)); size_t addedSize = 0; size_t remainingSize = frameSize; diff --git a/vrs/test/MultiRecordFileReaderTest.cpp b/vrs/test/MultiRecordFileReaderTest.cpp index c437f35c..37b23b86 100644 --- a/vrs/test/MultiRecordFileReaderTest.cpp +++ b/vrs/test/MultiRecordFileReaderTest.cpp @@ -105,7 +105,7 @@ class TestRecordable : public Recordable { }; struct VrsFileBuilder { - VrsFileBuilder(const string& path) : path_(path) { + explicit VrsFileBuilder(string path) : path_{std::move(path)} { XR_CHECK_FALSE(os::isFile(path_)); fileWriter_.addRecordable(&recordable_); recordable_.setRecordableIsActive(true); @@ -195,8 +195,7 @@ void assertEmptyStreamTags(const MultiRecordFileReader& reader) { class TestStreamPlayer : public StreamPlayer { public: - virtual bool processRecordHeader(const CurrentRecord& record, DataReference& outDataReference) - override { + bool processRecordHeader(const CurrentRecord&, DataReference&) override { return true; } @@ -547,7 +546,7 @@ TEST_F(MultiRecordFileReaderTest, getFirstAndLastRecord) { /// internally and match the record counts of each type class StreamIdCollisionTester { public: - StreamIdCollisionTester() : filePaths_(getOsTempPaths(kFilePathCount)), totalRecordCount_(0) { + StreamIdCollisionTester() : filePaths_(getOsTempPaths(kFilePathCount)) { for (size_t i = 0; i < filePaths_.size(); i++) { auto& filePath = filePaths_[i]; XR_CHECK_FALSE(os::isFile(filePath)); @@ -640,7 +639,7 @@ class StreamIdCollisionTester { recordable.setTag(kOriginalStreamIdTag, recordable.getStreamId().getName()); } - string getExpectedRecordCountTagKey(Record::Type type) const { + static string getExpectedRecordCountTagKey(Record::Type type) { return fmt::format("{}{}", kExpectedRecordCountTagPrefix, Record::typeName(type)); } @@ -749,7 +748,7 @@ class StreamIdCollisionTester { RecordFileWriter fileWriters_[kFilePathCount]; vector filePaths_; - size_t totalRecordCount_; + size_t totalRecordCount_{}; MultiRecordFileReader reader_; }; @@ -787,7 +786,7 @@ TEST_F(MultiRecordFileReaderTest, getFileChunks) { const auto fileChunks = multiReader.getFileChunks(); ASSERT_EQ(filePaths.size(), fileChunks.size()); for (size_t i = 0; i < filePaths.size(); i++) { - const auto fileChunk = fileChunks[i]; + const auto& fileChunk = fileChunks[i]; ASSERT_EQ(filePaths[i], fileChunk.first); ASSERT_EQ(expectedSize, fileChunk.second); } diff --git a/vrs/test/RecordFormatFileTest.cpp b/vrs/test/RecordFormatFileTest.cpp index 49047ad8..5c1d0f84 100644 --- a/vrs/test/RecordFormatFileTest.cpp +++ b/vrs/test/RecordFormatFileTest.cpp @@ -40,8 +40,8 @@ using namespace vrs::datalayout_conventions; namespace { // Some not so-nice strings to verify they are saved & restored as provided... -static const string kBadString1 = "\0x00hello\0x00"; -static const string kBadString2 = "\t1PASH3T1RS8113\n"; +const string kBadString1 = "\0x00hello\0x00"; +const string kBadString2 = "\t1PASH3T1RS8113\n"; // The SantaCruzCamera definitions were copied from the real SantaCruz definitions, // which aren't visible from the VRS module. @@ -51,18 +51,16 @@ namespace SantaCruzCamera { using ::vrs::AutoDataLayout; using ::vrs::AutoDataLayoutEnd; -using ::vrs::Bool; using ::vrs::DataPieceArray; using ::vrs::DataPieceValue; using ::vrs::DataReference; using ::vrs::Point3Df; using ::vrs::Point4Df; -using ::vrs::StreamPlayer; using ::vrs::datalayout_conventions::ImageSpecType; using ::vrs::FileFormat::LittleEndian; -static const int32_t kCalibrationDataSize = 22; -static const uint32_t kConfigurationVersion = 5; +const int32_t kCalibrationDataSize = 22; +const uint32_t kConfigurationVersion = 5; #pragma pack(push, 1) @@ -102,7 +100,7 @@ struct VRSDataV4 : VRSDataV3 { LittleEndian exposureDuration; }; -static constexpr float kGainMultiplierConvertor = 16.0; +constexpr float kGainMultiplierConvertor = 16.0; struct VRSDataV5 : VRSDataV4 { enum : uint32_t { kVersion = 5 }; @@ -149,7 +147,7 @@ struct VRSData : VRSDataV5 { }; struct VRSConfiguration { - VRSConfiguration() {} + VRSConfiguration() = default; LittleEndian width; LittleEndian height; @@ -236,11 +234,11 @@ class VariableImageSpec : public AutoDataLayout { AutoDataLayoutEnd end; }; -static const uint32_t kVariableImageRecordFormatVersion = 100; +const uint32_t kVariableImageRecordFormatVersion = 100; class OtherRecordable : public Recordable { public: - OtherRecordable(uint32_t cameraId) : Recordable(RecordableTypeId::UnitTest2) { + explicit OtherRecordable(uint32_t cameraId) : Recordable(RecordableTypeId::UnitTest2) { addRecordFormat( Record::Type::CONFIGURATION, SantaCruzCamera::DataLayoutConfiguration::kVersion, @@ -316,8 +314,8 @@ class DataLayoutFileTest : public Recordable, RecordFormatStreamPlayer { const size_t kFrameCount = 10; // count for each frame type public: - DataLayoutFileTest(string fileName) - : Recordable(RecordableTypeId::UnitTest1), fileName_(fileName) {} + explicit DataLayoutFileTest(string fileName) + : Recordable(RecordableTypeId::UnitTest1), fileName_{std::move(fileName)} {} const Record* createStateRecord() override { return createRecord(kTime, Record::Type::STATE, kStateVersion); @@ -758,10 +756,10 @@ class DataLayoutFileTest : public Recordable, RecordFormatStreamPlayer { private: string fileName_; - size_t unsupportedBlockCount_; - uint32_t configurationCount_; - uint32_t fixedImageCount_; - uint32_t variableImageCount_; + size_t unsupportedBlockCount_{}; + uint32_t configurationCount_{}; + uint32_t fixedImageCount_{}; + uint32_t variableImageCount_{}; VariableImageSpec imageSpec_; bool usesCompression{false}; }; @@ -781,7 +779,7 @@ struct DataLayoutFileTester : testing::Test { TEST_F(DataLayoutFileTester, createAndReadDataLayoutFile) { // arvr::logging::Channel::setGlobalLevel(arvr::logging::Level::Debug); - time_t timeBefore = time(NULL); + time_t timeBefore = time(nullptr); ASSERT_EQ(recordable.createVrsFile(), 0); EXPECT_EQ(recordable.readVrsFile(timeBefore), 0); diff --git a/vrs/test/RecordFormatTest.cpp b/vrs/test/RecordFormatTest.cpp index b0a60a90..20df8c38 100644 --- a/vrs/test/RecordFormatTest.cpp +++ b/vrs/test/RecordFormatTest.cpp @@ -65,7 +65,7 @@ class TestRecordable : public Recordable { } // namespace #define FORMAT_EQUAL(_block_format, _cstring) \ - EXPECT_STREQ(_block_format.asString().c_str(), _cstring) + EXPECT_STREQ((_block_format).asString().c_str(), _cstring) bool checkImageDimensions( const ContentBlock& cb, @@ -595,8 +595,8 @@ TEST_F(RecordFormatTest, testGetFormatVersionFromTagName) { const Record::Type STATE = Record::Type::STATE; const Record::Type DATA = Record::Type::DATA; - Record::Type recordType; - uint32_t formatVersion; + Record::Type recordType{}; + uint32_t formatVersion = 0; TEST_RECORD_FORMAT_NAME("RF:Data:0", DATA, 0); TEST_RECORD_FORMAT_NAME("RF:Data:00", DATA, 0); TEST_RECORD_FORMAT_NAME("RF:Data:1", DATA, 1); @@ -734,13 +734,13 @@ TEST_F(RecordFormatTest, testTagSetHelpers) { EXPECT_TRUE(tag_conventions::parseTagSet(jsonTags, readTags)); EXPECT_EQ(tags, readTags); - tags.push_back("hello"); + tags.emplace_back("hello"); jsonTags = tag_conventions::makeTagSet(tags); EXPECT_EQ(strcmp(jsonTags.c_str(), "{\"tags\":[\"hello\"]}"), 0); EXPECT_TRUE(tag_conventions::parseTagSet(jsonTags, readTags)); EXPECT_EQ(tags, readTags); - tags.push_back(jsonTags); + tags.emplace_back(jsonTags); jsonTags = tag_conventions::makeTagSet(tags); EXPECT_TRUE(tag_conventions::parseTagSet(jsonTags, readTags)); EXPECT_EQ(tags, readTags); diff --git a/vrs/test/RecordTest.cpp b/vrs/test/RecordTest.cpp index e8f6f83f..86afa03b 100644 --- a/vrs/test/RecordTest.cpp +++ b/vrs/test/RecordTest.cpp @@ -35,7 +35,7 @@ struct RecordTester : testing::Test {}; size_t collect( RecordFileWriter::RecordBatches& batches, - vector> recordManagers, + const vector>& recordManagers, double maxTime) { batches.emplace_back(new RecordFileWriter::RecordBatch()); RecordFileWriter::RecordBatch& batch = *batches.back(); @@ -292,7 +292,7 @@ static uint8_t f(uint8_t k) { const size_t kSize = 9; // odd, to expose padding issues union ArrayUnion { - ArrayUnion() {} // required + ArrayUnion() {} // required, do not use '= default' which would initialize the fields! Record::uninitialized_byte uninitialized_bytes[kSize]; uint8_t initialized_bytes[kSize]; }; @@ -304,28 +304,28 @@ TEST_F(RecordTester, initRecordTest) { // init reserved capacity to our pattern uint8_t* b = &buffer[0].byte; size_t initCapacity = buffer.capacity(); - for (uint8_t k = 0; k < initCapacity; k++) { + for (size_t k = 0; k < initCapacity; k++) { b[k] = f(k); } // allocate & verify that the buffer data wasn't initialized (still our pattern) buffer.resize(0); buffer.resize(10); const uint8_t* b1 = &buffer[0].byte; - for (uint8_t k = 0; k < initCapacity; k++) { + for (size_t k = 0; k < initCapacity; k++) { EXPECT_EQ(b1[k], f(k)); } // allocate & verify that the buffer data wasn't initialized (still our pattern) buffer.resize(0); buffer.resize(30); const uint8_t* b2 = &buffer[0].byte; - for (uint8_t k = 0; k < initCapacity; k++) { + for (size_t k = 0; k < initCapacity; k++) { EXPECT_EQ(b2[k], f(k)); } buffer.resize(0); buffer.resize(2000); // we should get a new buffer which data should be different const uint8_t* b3 = &buffer[0].byte; bool differentData = false; - for (uint8_t k = 0; !differentData && k < initCapacity; k++) { + for (size_t k = 0; !differentData && k < initCapacity; k++) { if (b3[k] != f(k)) { differentData = true; } @@ -343,7 +343,8 @@ TEST_F(RecordTester, initRecordTest) { namespace { class TestRecordable : public Recordable { public: - TestRecordable(RecordableTypeId typeId = RecordableTypeId::UnitTest1) : Recordable(typeId) {} + explicit TestRecordable(RecordableTypeId typeId = RecordableTypeId::UnitTest1) + : Recordable(typeId) {} const Record* createConfigurationRecord() override { return nullptr; } diff --git a/vrs/test/file_tests/ChunkedFileTest.cpp b/vrs/test/file_tests/ChunkedFileTest.cpp index b6ae322b..49e8f7cc 100644 --- a/vrs/test/file_tests/ChunkedFileTest.cpp +++ b/vrs/test/file_tests/ChunkedFileTest.cpp @@ -32,7 +32,7 @@ using namespace vrs; using coretech::getTestDataDir; namespace { -int addPies(DiskFile& file, string path) { +int addPies(DiskFile& file, const string& path) { int status = file.create(path); EXPECT_EQ(status, 0); if (status != 0) { diff --git a/vrs/test/file_tests/DeviceSimulatorTest.cpp b/vrs/test/file_tests/DeviceSimulatorTest.cpp index 18c19f84..c5026231 100644 --- a/vrs/test/file_tests/DeviceSimulatorTest.cpp +++ b/vrs/test/file_tests/DeviceSimulatorTest.cpp @@ -144,10 +144,10 @@ TEST_F(DeviceSimulator, preallocateTooManyIndex) { } struct ChunkCollector : public NewChunkHandler { - ChunkCollector(map& _chunks) : chunks{_chunks} { + explicit ChunkCollector(map& _chunks) : chunks{_chunks} { chunks.clear(); } - void newChunk(const string& path, size_t index, bool isLastChunk) override { + void newChunk(const string& path, size_t index, bool /*isLastChunk*/) override { EXPECT_EQ(chunks.find(index), chunks.end()); chunks[index] = path; } @@ -156,7 +156,7 @@ struct ChunkCollector : public NewChunkHandler { void checkChunks(const map& chunks, const string& path, size_t count) { EXPECT_EQ(chunks.size(), count); - if (chunks.size() > 0) { + if (!chunks.empty()) { auto iter = chunks.begin(); EXPECT_STREQ(iter->second.c_str(), path.c_str()); EXPECT_EQ(iter->first, 0); diff --git a/vrs/test/file_tests/FileCacheTest.cpp b/vrs/test/file_tests/FileCacheTest.cpp index 9f0a44ea..2324b811 100644 --- a/vrs/test/file_tests/FileCacheTest.cpp +++ b/vrs/test/file_tests/FileCacheTest.cpp @@ -37,7 +37,7 @@ struct FileCacheTest : testing::Test {}; } // namespace TEST_F(FileCacheTest, cacheTest) { - const string mainFolder = os::getTempFolder(); + const string& mainFolder = os::getTempFolder(); const string cacheName = "unit_test_vrs_file_cache"; ASSERT_EQ(FileCache::makeFileCache(cacheName, mainFolder), 0); FileCache* fcache = FileCache::getFileCache(); @@ -58,7 +58,7 @@ TEST_F(FileCacheTest, cacheTest) { } TEST_F(FileCacheTest, cacheDomainTest) { - const string mainFolder = os::getTempFolder(); + const string& mainFolder = os::getTempFolder(); const string cacheName = "unit_test_vrs_file_cache"; ASSERT_EQ(FileCache::makeFileCache(cacheName, mainFolder), 0); FileCache* fcache = FileCache::getFileCache(); @@ -98,7 +98,7 @@ void verifyDetails( map fileTags; map streamTags; vector recordIndex; - bool hasProperIndex; + bool hasProperIndex = false; int readStatus = FileDetailsCache::read( cacheFile, streamIds, fileTags, streamTags, recordIndex, hasProperIndex); if (readStatus == 0) { @@ -152,7 +152,7 @@ TEST_F(FileCacheTest, detailsTest) { threads.reserve(kThreadCount); ThreadParam params{cacheFile, reader, false}; for (uint32_t threadIndex = 0; threadIndex < kThreadCount; threadIndex++) { - threads.push_back(thread{createRecordsThreadTask, ¶ms}); + threads.emplace_back(createRecordsThreadTask, ¶ms); } for (uint32_t threadIndex = 0; threadIndex < kThreadCount; threadIndex++) { threads[threadIndex].join(); diff --git a/vrs/test/file_tests/RecordableTest.cpp b/vrs/test/file_tests/RecordableTest.cpp index 23189ac2..76219f4b 100644 --- a/vrs/test/file_tests/RecordableTest.cpp +++ b/vrs/test/file_tests/RecordableTest.cpp @@ -35,16 +35,16 @@ namespace { // Frame 0 is a random noise, just large enough to make that we attempt to compress it // leading to compressed data larger than the source data -static const uint32_t kFrame0Size = 320 * 240; -static vector sFrame0; +const uint32_t kFrame0Size = 320 * 240; +vector sFrame0; // Compress using different presets -static vector sCompression = { +vector sCompression = { vrs::CompressionPreset::Lz4Fast, vrs::CompressionPreset::ZstdFast, vrs::CompressionPreset::ZstdLight}; -static size_t sCompressionIndex = 0; +size_t sCompressionIndex = 0; void initFrame0() { default_random_engine generator; @@ -211,7 +211,7 @@ class RecordableTest : public Recordable, StreamPlayer { } } - static const char* getTag(const map& tags, string name) { + static const char* getTag(const map& tags, const string& name) { auto iter = tags.find(name); if (iter != tags.end()) { return iter->second.c_str(); @@ -367,7 +367,7 @@ class RecordableTest : public Recordable, StreamPlayer { return filePlayer.closeFile(); } - int rebuildIndex(const string& fileName) { + static int rebuildIndex(const string& fileName) { RecordFileReader filePlayer; RETURN_ON_FAILURE(filePlayer.openFile(fileName)); EXPECT_TRUE(filePlayer.hasIndex()); @@ -476,8 +476,8 @@ class RecordableTest : public Recordable, StreamPlayer { } vector threads; for (uint32_t threadIndex = 0; threadIndex < kThreadCount; threadIndex++) { - threads.push_back( - thread{&RecordableTest::createRecordsThreadTask, this, &threadParams[threadIndex]}); + threads.emplace_back( + &RecordableTest::createRecordsThreadTask, this, &threadParams[threadIndex]); } for (uint32_t threadIndex = 0; threadIndex < kThreadCount; threadIndex++) { threads[threadIndex].join(); @@ -536,7 +536,7 @@ class RecordableTest : public Recordable, StreamPlayer { return fileWriter.waitForFileClosed(); } - void checkShortFile(const string& fileName) { + static void checkShortFile(const string& fileName) { RecordFileReader file; EXPECT_EQ(file.openFile(fileName), 0); EXPECT_EQ(file.getStreams().size(), 1); @@ -553,7 +553,7 @@ class RecordableTest : public Recordable, StreamPlayer { : kFrameWidth * kFrameHeight - (frameNumber % 200); } - uint8_t getByteOfFrame(uint32_t frameNumber, uint32_t byteNumber) { + static uint8_t getByteOfFrame(uint32_t frameNumber, uint32_t byteNumber) { if (frameNumber == 0) { return sFrame0[byteNumber]; } @@ -566,7 +566,7 @@ class RecordableTest : public Recordable, StreamPlayer { private: string typeName_; - uint32_t frameNumber_; + uint32_t frameNumber_{}; vector readBuffer_; double lastTimestamp_ = -DBL_MAX; }; @@ -641,7 +641,7 @@ TEST_F(RecordableTester, ReuseRecordFileWriter) { string testFilePath = os::getTempFolder() + "RecordableTest-f.vrs"; RecordFileWriter fileWriter; ASSERT_EQ(recordable.createShortFile(fileWriter, testFilePath), 0); - recordable.checkShortFile(testFilePath); + RecordableTest::checkShortFile(testFilePath); ASSERT_EQ(recordable.createShortFile(fileWriter, testFilePath), 0); - recordable.checkShortFile(testFilePath); + RecordableTest::checkShortFile(testFilePath); } diff --git a/vrs/test/file_tests/SimpleFileHandlerTest.cpp b/vrs/test/file_tests/SimpleFileHandlerTest.cpp index c1924a24..04f6e9b1 100644 --- a/vrs/test/file_tests/SimpleFileHandlerTest.cpp +++ b/vrs/test/file_tests/SimpleFileHandlerTest.cpp @@ -35,19 +35,19 @@ using namespace vrs; namespace { -static const uint32_t kFrameWidth = 640; -static const uint32_t kFrameHeight = 480; -static const uint32_t kPixelByteSize = 1; +const uint32_t kFrameWidth = 640; +const uint32_t kFrameHeight = 480; +const uint32_t kPixelByteSize = 1; -static const uint32_t kFrameRate = 30; // Hz -static const uint32_t kImagesPerTimestamp = 4; // to test ordering of records with identical tstamps -static const uint32_t kFrameCount = kFrameRate * 5; // 5 seconds worth of frames -static const double kInterFrameDelay = 1.0 / kFrameRate; +const uint32_t kFrameRate = 30; // Hz +const uint32_t kImagesPerTimestamp = 4; // to test ordering of records with identical tstamps +const uint32_t kFrameCount = kFrameRate * 5; // 5 seconds worth of frames +const double kInterFrameDelay = 1.0 / kFrameRate; -static const uint32_t kConfigurationVersion = 1; -static const uint32_t kDataVersion = 1; +const uint32_t kConfigurationVersion = 1; +const uint32_t kDataVersion = 1; -static const double kStartTimestamp = 1543864285; +const double kStartTimestamp = 1543864285; using datalayout_conventions::ImageSpecType; @@ -139,10 +139,10 @@ class ImageStream : public Recordable { class ImageStreamPlayer : public RecordFormatStreamPlayer { public: - virtual bool onDataLayoutRead(const CurrentRecord& record, size_t blockIndex, DataLayout& dl) { + bool onDataLayoutRead(const CurrentRecord& record, size_t blockIndex, DataLayout& dl) override { if (record.recordType == Record::Type::DATA) { ImageStreamMetaData& data = getExpectedLayout(dl, blockIndex); - uint64_t frameCounter; + uint64_t frameCounter = 0; bool foundFrameCounter = data.frameCounter.get(frameCounter); EXPECT_TRUE(foundFrameCounter); if (foundFrameCounter) { @@ -158,7 +158,7 @@ class ImageStreamPlayer : public RecordFormatStreamPlayer { struct SimpleFileHandlerTest : testing::Test { // Demonstrate how to create a VRS file, // holding all the records in memory before writing them all in a single call. - int createFileAtOnce(const string& filePath) { + static int createFileAtOnce(const string& filePath) { // Create a container to hold all references to all the streams we want to record RecordFileWriter fileWriter; @@ -189,7 +189,7 @@ struct SimpleFileHandlerTest : testing::Test { // Demonstrate how to create a VRS file, // writing records in the background as we create more. // Even if all your records fit in memory, this version is much faster, so you should use it! - int createFileStreamingToDisk(const string& filePath) { + static int createFileStreamingToDisk(const string& filePath) { // Create a container to hold all references to all the streams we want to record RecordFileWriter fileWriter; @@ -227,7 +227,7 @@ struct SimpleFileHandlerTest : testing::Test { return 0; } - void checkFileHandler(const string& filePath) { + static void checkFileHandler(const string& filePath) { // Verify that the file was created, and looks like we think it should RecordFileReader reader; int openFileStatus = reader.openFile(filePath); diff --git a/vrs/test/file_tests/UserRecordsFileHandlerTest.cpp b/vrs/test/file_tests/UserRecordsFileHandlerTest.cpp index bcd61652..a7d2d59a 100644 --- a/vrs/test/file_tests/UserRecordsFileHandlerTest.cpp +++ b/vrs/test/file_tests/UserRecordsFileHandlerTest.cpp @@ -34,7 +34,7 @@ namespace { struct UserRecordsFileHandlerTest : testing::Test {}; -const string kUserFileRecordsFileHandler = "UserRecordsFileHandler"; +string kUserFileRecordsFileHandler = "UserRecordsFileHandler"; #define METHOD_NOT_SUPPORTED() \ throw std::runtime_error( \ @@ -42,7 +42,7 @@ const string kUserFileRecordsFileHandler = "UserRecordsFileHandler"; class UserRecordsFileHandler : public WriteFileHandler { public: - UserRecordsFileHandler() {} + UserRecordsFileHandler() = default; std::unique_ptr makeNew() const override { return make_unique(); } @@ -112,7 +112,7 @@ class UserRecordsFileHandler : public WriteFileHandler { bool isRemoteFileSystem() const override { return false; } - int parseUri(FileSpec& intOutFileSpec, size_t colonIndex) const override { + int parseUri(FileSpec& intOutFileSpec, size_t /*colonIndex*/) const override { intOutFileSpec.chunks.resize(1); return FileSpec::parseUri( intOutFileSpec.uri, diff --git a/vrs/test/helpers/TestProcess.cpp b/vrs/test/helpers/TestProcess.cpp index cdef1696..c121f106 100644 --- a/vrs/test/helpers/TestProcess.cpp +++ b/vrs/test/helpers/TestProcess.cpp @@ -46,7 +46,7 @@ inline string trim(const string& line) { return vrs::helpers::trim(line, " \t\r\n"); } -bool TestProcess::start(string arg, bp::ipstream* sout) { +bool TestProcess::start(const string& arg, bp::ipstream* sout) { string path = string(processName_); if (XR_VERIFY(findBinary(path))) { #if IS_VRS_FB_INTERNAL() @@ -55,16 +55,17 @@ bool TestProcess::start(string arg, bp::ipstream* sout) { } #endif if (sout != nullptr) { - process.reset(new bp::child(path + ' ' + arg, bp::std_out > *sout, bp::std_err > bp::null)); + process = + make_unique(path + ' ' + arg, bp::std_out > *sout, bp::std_err > bp::null); } else { - process.reset(new bp::child(path + ' ' + arg)); + process = make_unique(path + ' ' + arg); } return true; } return false; } -string TestProcess::getJsonOutput(bp::ipstream& output) const { +string TestProcess::getJsonOutput(bp::ipstream& output) { string line; while (getline(output, line)) { line = trim(line); diff --git a/vrs/test/helpers/TestProcess.h b/vrs/test/helpers/TestProcess.h index d5d357d2..8efbdab5 100644 --- a/vrs/test/helpers/TestProcess.h +++ b/vrs/test/helpers/TestProcess.h @@ -40,13 +40,13 @@ namespace bp = boost::process; class TestProcess { public: - TestProcess(const char* processName) : processName_{processName} {} - bool start(std::string arg, bp::ipstream& sout) { + explicit TestProcess(const char* processName) : processName_{processName} {} + bool start(const std::string& arg, bp::ipstream& sout) { return start(arg, &sout); } - bool start(std::string arg, bp::ipstream* sout = nullptr); + bool start(const std::string& arg, bp::ipstream* sout = nullptr); - std::string getJsonOutput(bp::ipstream& output) const; + static std::string getJsonOutput(bp::ipstream& output); int runProcess(); @@ -54,9 +54,9 @@ class TestProcess { std::unique_ptr process; const char* processName_; - bool looksLikeAFbCentOSServer(); + static bool looksLikeAFbCentOSServer(); - bool findBinary(std::string& inOutName); + static bool findBinary(std::string& inOutName); }; } // namespace test