Introduce optimized CRC-32 implementation

Os/File.cpp

@@ -4,11 +4,9 @@
 // ======================================================================
 #include <Fw/Types/Assert.hpp>
 #include <Os/File.hpp>
+#include <Utils/Hash/Hash.hpp>
 #include <algorithm>
 
-extern "C" {
-#include <Utils/Hash/libcrc/lib_crc.h>  // borrow CRC
-}
 namespace Os {
 
 File::File() : m_crc_buffer(), m_handle_storage(), m_delegate(*FileInterface::getDelegate(m_handle_storage)) {
@@ -229,7 +227,7 @@ File::Status File::calculateCrc(U32& crc) {
 
 File::Status File::incrementalCrc(FwSizeType& size) {
     File::Status status = File::Status::OP_OK;
-    FW_ASSERT(size <= FW_FILE_CHUNK_SIZE);
+    FW_ASSERT(size <= FW_FILE_CHUNK_SIZE, FwAssertArgType(size));
     if (OPEN_NO_MODE == this->m_mode) {
         status = File::Status::NOT_OPENED;
     } else if (OPEN_READ != this->m_mode) {
@@ -238,9 +236,14 @@ File::Status File::incrementalCrc(FwSizeType& size) {
         // Read data without waiting for additional data to be available
         status = this->read(this->m_crc_buffer, size, File::WaitType::NO_WAIT);
         if (OP_OK == status) {
-            for (FwSizeType i = 0; i < size && i < FW_FILE_CHUNK_SIZE; i++) {
-                this->m_crc = static_cast<U32>(update_crc_32(this->m_crc, static_cast<CHAR>(this->m_crc_buffer[i])));
-            }
+            FW_ASSERT(size <= FW_FILE_CHUNK_SIZE, FwAssertArgType(size));
+            // FIXME: Utils::Hash should be integrated more carefully into File
+            Utils::Hash hash;
+            hash.setHashValue(U32(~this->m_crc));
+            hash.update(this->m_crc_buffer, size);
+            U32 crc;
+            hash.finalize(crc);
+            this->m_crc = ~crc;
         }
     }
     return status;

Os/test/ut/file/FileRules.cpp

@@ -6,9 +6,7 @@
 #include <cstdio>
 #include "RulesHeaders.hpp"
 #include "STest/Pick/Pick.hpp"
-extern "C" {
-#include <Utils/Hash/libcrc/lib_crc.h>  // borrow CRC
-}
+#include "Utils/Hash/Hash.hpp"
 
 // For testing, limit files to 32K
 const FwSizeType FILE_DATA_MAXIMUM = 32 * 1024;
@@ -82,14 +80,22 @@ void Os::Test::FileTest::Tester::shadow_flush() {
 }
 
 void Os::Test::FileTest::Tester::shadow_crc(U32& crc) {
-    crc = this->m_independent_crc;
+    Utils::Hash hash;
+    hash.setHashValue(U32(~this->m_independent_crc));
+
     SyntheticFileData& data = *reinterpret_cast<SyntheticFileData*>(this->m_shadow.getHandle());
 
     // Calculate CRC on full file starting at m_pointer
     for (FwSizeType i = data.m_pointer; i < data.m_data.size();
          i++, this->m_shadow.seek(1, Os::File::SeekType::RELATIVE)) {
-        crc = update_crc_32(crc, static_cast<char>(data.m_data.at(i)));
+        U8 byte = data.m_data.at(i);
+        hash.update(&byte, sizeof(byte));
     }
+
+    U32 crcFinal;
+    hash.finalize(crcFinal);
+    crc = ~crcFinal;
+
     // Update tracking variables
     this->m_independent_crc = Os::File::INITIAL_CRC;
 }
@@ -102,7 +108,16 @@ void Os::Test::FileTest::Tester::shadow_partial_crc(FwSizeType& size) {
         std::min(static_cast<FwSizeType>(data.m_pointer) + size, static_cast<FwSizeType>(data.m_data.size()));
     size = (data.m_pointer >= bound) ? 0 : static_cast<FwSizeType>(bound - data.m_pointer);
     for (FwSizeType i = data.m_pointer; i < bound; i++) {
-        this->m_independent_crc = update_crc_32(this->m_independent_crc, static_cast<char>(data.m_data.at(i)));
+        U8 byte = data.m_data.at(i);
+
+        Utils::Hash hash;
+        hash.setHashValue(U32(~this->m_independent_crc));
+        hash.update(&byte, sizeof(byte));
+
+        U32 crcFinal;
+        hash.finalize(crcFinal);
+        this->m_independent_crc = ~crcFinal;
+
         this->m_shadow.seek(1, Os::File::SeekType::RELATIVE);
     }
 }

Svc/Ccsds/Utils/CRC16.hpp

@@ -2,7 +2,7 @@
 #define SVC_CCSDS_UTILS_CRC16_HPP
 
 #include "Fw/Types/BasicTypes.hpp"
-// Include the lic crc c library:
+// Include the libcrc library because we need update_crc_ccitt that is not provided through the main interface.
 extern "C" {
 #include <Utils/Hash/libcrc/lib_crc.h>
 }

Svc/CmdSequencer/CmdSequencerImpl.cpp

@@ -12,9 +12,7 @@
 #include <Fw/Types/SerialBuffer.hpp>
 #include <Fw/Types/Serializable.hpp>
 #include <Svc/CmdSequencer/CmdSequencerImpl.hpp>
-extern "C" {
-#include <Utils/Hash/libcrc/lib_crc.h>
-}
+#include <Utils/Hash/Hash.hpp>
 
 namespace Svc {
 

Svc/CmdSequencer/CmdSequencerImpl.hpp

@@ -11,6 +11,7 @@
 #ifndef Svc_CmdSequencerImpl_HPP
 #define Svc_CmdSequencerImpl_HPP
 
+#include <Utils/Hash/Hash.hpp>
 #include "Fw/Com/ComBuffer.hpp"
 #include "Fw/Types/MemAllocator.hpp"
 #include "Os/File.hpp"
@@ -297,11 +298,8 @@ class CmdSequencerComponentImpl final : public CmdSequencerComponentBase {
                         FwSizeType bufferSize  //!< The buffer size
             );
 
-            //! Finalize computed CRC
-            void finalize();
-
             //! Computed CRC
-            U32 m_computed;
+            Utils::Hash m_computed;
 
             //! Stored CRC
             U32 m_stored;

Svc/CmdSequencer/FPrimeSequence.cpp

@@ -10,36 +10,29 @@
 
 #include "Fw/Types/Assert.hpp"
 #include "Svc/CmdSequencer/CmdSequencerImpl.hpp"
-extern "C" {
-#include "Utils/Hash/libcrc/lib_crc.h"
-}
 
 namespace Svc {
 
-CmdSequencerComponentImpl::FPrimeSequence::CRC ::CRC() : m_computed(INITIAL_COMPUTED_VALUE), m_stored(0) {}
+CmdSequencerComponentImpl::FPrimeSequence::CRC ::CRC() : m_computed(), m_stored(0) {}
 
 void CmdSequencerComponentImpl::FPrimeSequence::CRC ::init() {
-    this->m_computed = INITIAL_COMPUTED_VALUE;
+    this->m_computed.init();
 }
 
 void CmdSequencerComponentImpl::FPrimeSequence::CRC ::update(const BYTE* buffer, FwSizeType bufferSize) {
     FW_ASSERT(buffer);
-    for (FwSizeType index = 0; index < bufferSize; index++) {
-        this->m_computed = static_cast<U32>(update_crc_32(this->m_computed, static_cast<char>(buffer[index])));
-    }
-}
-
-void CmdSequencerComponentImpl::FPrimeSequence::CRC ::finalize() {
-    this->m_computed = ~this->m_computed;
+    this->m_computed.update(buffer, bufferSize);
 }
 
 CmdSequencerComponentImpl::FPrimeSequence ::FPrimeSequence(CmdSequencerComponentImpl& component)
     : Sequence(component) {}
 
 bool CmdSequencerComponentImpl::FPrimeSequence ::validateCRC() {
     bool result = true;
-    if (this->m_crc.m_stored != this->m_crc.m_computed) {
-        this->m_events.fileCRCFailure(this->m_crc.m_stored, this->m_crc.m_computed);
+    U32 computed;
+    this->m_crc.m_computed.finalize(computed);
+    if (this->m_crc.m_stored != computed) {
+        this->m_events.fileCRCFailure(this->m_crc.m_stored, computed);
         result = false;
     }
     return result;
@@ -104,7 +97,6 @@
     if (status) {
         const FwSizeType buffLen = this->m_buffer.getSize();
         this->m_crc.update(buffAddr, buffLen);
-        this->m_crc.finalize();
     }
     return status;
 }

Svc/CmdSequencer/formats/AMPCSSequence.cpp

@@ -10,12 +10,10 @@
 // ======================================================================
 
 #include "Svc/CmdSequencer/formats/AMPCSSequence.hpp"
+#include <Utils/Hash/Hash.hpp>
 #include "Fw/Com/ComPacket.hpp"
 #include "Fw/Types/Assert.hpp"
 #include "Os/FileSystem.hpp"
-extern "C" {
-#include "Utils/Hash/libcrc/lib_crc.h"
-}
 
 namespace Svc {
 
@@ -96,8 +94,10 @@ bool AMPCSSequence ::readSequenceFile(const Fw::ConstStringBase& seqFileName) {
 
 bool AMPCSSequence ::validateCRC() {
     bool result = true;
-    if (this->m_crc.m_stored != this->m_crc.m_computed) {
-        this->m_events.fileCRCFailure(this->m_crc.m_stored, this->m_crc.m_computed);
+    U32 computed;
+    this->m_crc.m_computed.finalize(computed);
+    if (this->m_crc.m_stored != computed) {
+        this->m_events.fileCRCFailure(this->m_crc.m_stored, computed);
         result = false;
     }
     return result;
@@ -190,7 +190,6 @@ bool AMPCSSequence ::readOpenSequenceFile() {
         FW_ASSERT(buffLen == this->m_header.m_fileSize, static_cast<FwAssertArgType>(buffLen),
                   static_cast<FwAssertArgType>(this->m_header.m_fileSize));
         this->m_crc.update(buffAddr, buffLen);
-        this->m_crc.finalize();
     }
     return status;
 }

Svc/CmdSequencer/test/ut/InvalidFiles.cpp

@@ -46,7 +46,9 @@ void CmdSequencerTester ::BadCRC() {
     // Assert events
     const CmdSequencerComponentImpl::FPrimeSequence::CRC& crc = file.getCRC();
     ASSERT_EVENTS_SIZE(1);
-    ASSERT_EVENTS_CS_FileCrcFailure(0, file.getName().toChar(), crc.m_stored, crc.m_computed);
+    U32 crcFinal;
+    crc.m_computed.finalize(crcFinal);
+    ASSERT_EVENTS_CS_FileCrcFailure(0, file.getName().toChar(), crc.m_stored, crcFinal);
     // Assert telemetry
     ASSERT_TLM_SIZE(1);
     ASSERT_TLM_CS_Errors(0, 1);

Svc/CmdSequencer/test/ut/SequenceFiles/AMPCS/CRCs.cpp

@@ -55,15 +55,16 @@ void writeFile(Os::File& file,    //!< The file
 void createFile(Fw::SerializeBufferBase& buffer, const char* const fileName) {
     CRC crc;
     computeCRC(buffer, crc);
-    writeCRC(crc.m_computed, fileName);
+    U32 crcFinal;
+    crc.m_computed.finalize(crcFinal);
+    writeCRC(crcFinal, fileName);
 }
 
 void computeCRC(Fw::SerializeBufferBase& buffer, CRC& crc) {
     crc.init();
     const U8* const addr = buffer.getBuffAddr();
     const U32 size = buffer.getSize();
     crc.update(addr, size);
-    crc.finalize();
 }
 
 void removeFile(const char* const fileName) {

Svc/CmdSequencer/test/ut/SequenceFiles/BadCRCFile.cpp

@@ -36,8 +36,9 @@ void BadCRCFile ::serializeFPrime(Fw::SerializeBufferBase& buffer) {
     const U32 size = buffer.getSize();
     this->crc.init();
     this->crc.update(addr, size);
-    this->crc.finalize();
-    crc.m_stored = this->crc.m_computed + 1;
+    U32 crcFinal;
+    this->crc.m_computed.finalize(crcFinal);
+    crc.m_stored = crcFinal + 1;
     ASSERT_EQ(Fw::FW_SERIALIZE_OK, buffer.serializeFrom(this->crc.m_stored));
 }
 
@@ -49,7 +50,9 @@ void BadCRCFile ::serializeAMPCS(Fw::SerializeBufferBase& buffer) {
     ASSERT_EQ(Fw::FW_SERIALIZE_OK, buffer.serializeFrom(recordData));
     // CRC
     AMPCS::CRCs::computeCRC(buffer, this->crc);
-    this->crc.m_stored = this->crc.m_computed + 1;
+    U32 crcFinal;
+    this->crc.m_computed.finalize(crcFinal);
+    crc.m_stored = crcFinal + 1;
     AMPCS::CRCs::writeCRC(this->crc.m_stored, this->getName().toChar());
 }
 

Svc/CmdSequencer/test/ut/SequenceFiles/FPrime/CRCs.cpp

@@ -23,8 +23,9 @@ void serialize(Fw::SerializeBufferBase& destBuffer) {
     CmdSequencerComponentImpl::FPrimeSequence::CRC crc;
     crc.init();
     crc.update(destBuffer.getBuffAddr(), destBuffer.getSize());
-    crc.finalize();
-    ASSERT_EQ(destBuffer.serializeFrom(crc.m_computed), Fw::FW_SERIALIZE_OK);
+    U32 crcFinal;
+    crc.m_computed.finalize(crcFinal);
+    ASSERT_EQ(destBuffer.serializeFrom(crcFinal), Fw::FW_SERIALIZE_OK);
 }
 
 }  // namespace CRCs

Svc/FpySequencer/FpySequencer.cpp

@@ -19,7 +19,7 @@ FpySequencer ::FpySequencer(const char* const compName)
       m_allocatorId(0),
       m_sequenceFilePath("<invalid_seq>"),
       m_sequenceObj(),
-      m_computedCRC(0),
+      m_computedCRC(),
       m_sequenceBlockState(),
       m_savedOpCode(0),
       m_savedCmdSeq(0),

Svc/FpySequencer/FpySequencer.hpp

@@ -21,6 +21,7 @@
 #include "Svc/FpySequencer/HeaderSerializableAc.hpp"
 #include "Svc/FpySequencer/SequenceSerializableAc.hpp"
 #include "Svc/FpySequencer/StatementSerializableAc.hpp"
+#include "Utils/Hash/Hash.hpp"
 #include "config/FppConstantsAc.hpp"
 
 static_assert(Svc::Fpy::MAX_SEQUENCE_ARG_COUNT <= std::numeric_limits<U8>::max(),
@@ -604,7 +605,7 @@ class FpySequencer : public FpySequencerComponentBase {
     // the sequence, loaded in memory
     Fpy::Sequence m_sequenceObj;
     // live running computation of CRC (updated as we read)
-    U32 m_computedCRC;
+    Utils::Hash m_computedCRC;
 
     // whether or not the sequence we're about to run should return immediately or
     // block on completion
@@ -709,11 +710,6 @@ class FpySequencer : public FpySequencerComponentBase {
     // Validation state
     // ----------------------------------------------------------------------
 
-    static void updateCrc(U32& crc,              //!< The CRC to update
-                          const U8* buffer,      //!< The buffer
-                          FwSizeType bufferSize  //!< The buffer size
-    );
-
     // loads the sequence in memory, and does header/crc/integrity checks.
     // return SUCCESS if sequence is valid, FAILURE otherwise
     Fw::Success validate();

Svc/FpySequencer/FpySequencerValidationState.cpp

@@ -1,8 +1,7 @@
+#include <Utils/Hash/Hash.hpp>
 #include "Svc/FpySequencer/FppConstantsAc.hpp"
 #include "Svc/FpySequencer/FpySequencer.hpp"
-extern "C" {
-#include "Utils/Hash/libcrc/lib_crc.h"
-}
+
 namespace Svc {
 
 void FpySequencer::allocateBuffer(FwEnumStoreType identifier, Fw::MemAllocator& allocator, FwSizeType bytes) {
@@ -24,21 +23,14 @@ void FpySequencer::deallocateBuffer(Fw::MemAllocator& allocator) {
     this->m_sequenceBuffer.clear();
 }
 
-void FpySequencer::updateCrc(U32& crc, const U8* buffer, FwSizeType bufferSize) {
-    FW_ASSERT(buffer);
-    for (FwSizeType index = 0; index < bufferSize; index++) {
-        crc = static_cast<U32>(update_crc_32(crc, static_cast<char>(buffer[index])));
-    }
-}
-
 // loads the sequence in memory, and does header/crc/integrity checks.
 // return SUCCESS if sequence is valid, FAILURE otherwise
 Fw::Success FpySequencer::validate() {
     FW_ASSERT(this->m_sequenceFilePath.length() > 0);
 
     // crc needs to be initialized with a particular value
     // for the calculation to work
-    this->m_computedCRC = CRC_INITIAL_VALUE;
+    this->m_computedCRC.init();
 
     Os::File sequenceFile;
     Os::File::Status openStatus = sequenceFile.open(this->m_sequenceFilePath.toChar(), Os::File::OPEN_READ);
@@ -179,10 +171,11 @@ Fw::Success FpySequencer::readFooter() {
     }
 
     // need this for some reason to "finalize" the crc TODO get an explanation on this
-    this->m_computedCRC = ~this->m_computedCRC;
+    U32 computedCRC;
+    this->m_computedCRC.finalize(computedCRC);
 
-    if (this->m_computedCRC != this->m_sequenceObj.get_footer().get_crc()) {
-        this->log_WARNING_HI_WrongCRC(this->m_sequenceObj.get_footer().get_crc(), this->m_computedCRC);
+    if (computedCRC != this->m_sequenceObj.get_footer().get_crc()) {
+        this->log_WARNING_HI_WrongCRC(this->m_sequenceObj.get_footer().get_crc(), computedCRC);
         return Fw::Success::FAILURE;
     }
 
@@ -229,7 +222,7 @@ Fw::Success FpySequencer::readBytes(Os::File& file,
     FW_ASSERT(serializeStatus == Fw::FW_SERIALIZE_OK, serializeStatus);
 
     if (updateCrc) {
-        FpySequencer::updateCrc(this->m_computedCRC, this->m_sequenceBuffer.getBuffAddr(), expectedReadLen);
+        this->m_computedCRC.update(this->m_sequenceBuffer.getBuffAddr(), expectedReadLen);
     }
 
     return Fw::Success::SUCCESS;