From 62e98107f760df9dce7dfcbc7edfc72b5b2830e0 Mon Sep 17 00:00:00 2001 From: devdl11 Date: Fri, 8 Apr 2022 13:49:44 +0200 Subject: [PATCH] [Reviews] Code correction and improvement --- bootloader/Makefile | 1 + bootloader/boot.cpp | 11 +++-------- bootloader/itoa.cpp | 2 +- bootloader/kernel_header.cpp | 13 ++++--------- bootloader/kernel_header.h | 2 +- bootloader/main.cpp | 4 ++-- bootloader/usb_data.cpp | 8 ++++---- bootloader/usb_data.h | 10 +++++----- bootloader/userland_header.cpp | 4 ++-- bootloader/userland_header.h | 8 ++++---- bootloader/utility.cpp | 10 ++++++++++ bootloader/{itoa.h => utility.h} | 3 ++- ion/src/device/bootloader/usb/calculator.h | 2 +- ion/src/device/bootloader/usb/dfu_interface.cpp | 6 +++--- ion/src/device/bootloader/usb/dfu_interface.h | 8 ++++---- .../stack/descriptor/device_capability_descriptor.h | 4 ++-- .../platform_device_capability_descriptor.h | 4 ++-- ion/src/device/n0110/drivers/board.cpp | 2 +- ion/src/device/shared/drivers/display.cpp | 2 +- ion/src/device/shared/drivers/reset.cpp | 2 +- ion/src/device/shared/drivers/wakeup.h | 2 +- ion/src/device/shared/regs/tim.h | 2 +- ion/src/device/shared/usb/calculator.h | 2 +- .../stack/descriptor/device_capability_descriptor.h | 4 ++-- .../platform_device_capability_descriptor.h | 4 ++-- 25 files changed, 61 insertions(+), 59 deletions(-) create mode 100644 bootloader/utility.cpp rename bootloader/{itoa.h => utility.h} (71%) diff --git a/bootloader/Makefile b/bootloader/Makefile index 41648a0e3..56648ddba 100644 --- a/bootloader/Makefile +++ b/bootloader/Makefile @@ -12,6 +12,7 @@ bootloader_src += $(addprefix bootloader/,\ recovery.cpp \ usb_data.cpp \ itoa.cpp \ + utility.cpp \ ) bootloader_images = $(addprefix bootloader/, \ diff --git a/bootloader/boot.cpp b/bootloader/boot.cpp index 292ac5c59..cc71d711d 100644 --- a/bootloader/boot.cpp +++ b/bootloader/boot.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include @@ -25,14 +26,8 @@ void Boot::bootSlot(Bootloader::Slot s) { // We are trying to boot epsilon, so we check the version and show an advertisement if needed const char * version = s.userlandHeader()->version(); const char * min = "18.2.4"; - int vsum = 0; - for (int i = 0; i < strlen(version); i++) { - vsum += version[i] * (100-i*15); - } - int minsum = 0; - for (int i = 0; i < strlen(min); i++) { - minsum += min[i] * (100-i*15); - } + int vsum = Bootloader::Utility::versionSum(version, strlen(version)); + int minsum = Bootloader::Utility::versionSum(min, strlen(min)); if (vsum >= minsum) { Interface::drawEpsilonAdvertisement(); uint64_t scan = 0; diff --git a/bootloader/itoa.cpp b/bootloader/itoa.cpp index 34aa60692..2afd28b02 100644 --- a/bootloader/itoa.cpp +++ b/bootloader/itoa.cpp @@ -1,6 +1,6 @@ #include #include -#include +#include // https://www.techiedelight.com/implement-itoa-function-in-c/ diff --git a/bootloader/kernel_header.cpp b/bootloader/kernel_header.cpp index 5b12d46f6..7f55b9fc3 100644 --- a/bootloader/kernel_header.cpp +++ b/bootloader/kernel_header.cpp @@ -1,4 +1,5 @@ #include +#include namespace Bootloader { @@ -22,16 +23,10 @@ const void(*KernelHeader::startPointer() const)() { return m_startPointer; } -const bool KernelHeader::isNewVersion() const { - int sum = 0; - for (int i = 0; i < 2; i++) { - sum += m_version[i] * (5 - i); - } +const bool KernelHeader::isAboveVersion16 () const { + int sum = Bootloader::Utility::versionSum(m_version, 2); char newVersion[] = "16"; - int min = 0; - for (int i = 0; i < 2; i++) { - min += newVersion[i] * (5 - i); - } + int min = Bootloader::Utility::versionSum(newVersion, 2); return sum >= min; } diff --git a/bootloader/kernel_header.h b/bootloader/kernel_header.h index c62c1590c..44963851d 100644 --- a/bootloader/kernel_header.h +++ b/bootloader/kernel_header.h @@ -10,7 +10,7 @@ public: const char * version() const; const char * patchLevel() const; const bool isValid() const; - const bool isNewVersion() const; + const bool isAboveVersion16() const; const uint32_t* stackPointer() const; const void(*startPointer() const)(); diff --git a/bootloader/main.cpp b/bootloader/main.cpp index 3b052bc51..c847ad882 100644 --- a/bootloader/main.cpp +++ b/bootloader/main.cpp @@ -20,7 +20,7 @@ __attribute__ ((noreturn)) void ion_main(int argc, const char * const argv[]) { bool isSlotA = Bootloader::Slot::A().kernelHeader()->isValid(); if (isSlotA) { - Bootloader::ExamMode::ExamMode SlotAExamMode = (Bootloader::ExamMode::ExamMode)Bootloader::ExamMode::SlotsExamMode::FetchSlotAExamMode(Bootloader::Slot::A().kernelHeader()->isNewVersion()); + Bootloader::ExamMode::ExamMode SlotAExamMode = (Bootloader::ExamMode::ExamMode)Bootloader::ExamMode::SlotsExamMode::FetchSlotAExamMode(Bootloader::Slot::A().kernelHeader()->isAboveVersion16()); if (SlotAExamMode != Bootloader::ExamMode::ExamMode::Off && SlotAExamMode != Bootloader::ExamMode::ExamMode::Unknown) { // We boot the slot in exam_mode Bootloader::Slot::A().boot(); @@ -30,7 +30,7 @@ __attribute__ ((noreturn)) void ion_main(int argc, const char * const argv[]) { bool isSlotB = Bootloader::Slot::B().kernelHeader()->isValid(); if (isSlotB) { - Bootloader::ExamMode::ExamMode SlotBExamMode = (Bootloader::ExamMode::ExamMode)Bootloader::ExamMode::SlotsExamMode::FetchSlotBExamMode(Bootloader::Slot::B().kernelHeader()->isNewVersion()); + Bootloader::ExamMode::ExamMode SlotBExamMode = (Bootloader::ExamMode::ExamMode)Bootloader::ExamMode::SlotsExamMode::FetchSlotBExamMode(Bootloader::Slot::B().kernelHeader()->isAboveVersion16()); if (SlotBExamMode != Bootloader::ExamMode::ExamMode::Off && SlotBExamMode != Bootloader::ExamMode::ExamMode::Unknown && isSlotB) { // We boot the slot in exam_mode Bootloader::Slot::B().boot(); diff --git a/bootloader/usb_data.cpp b/bootloader/usb_data.cpp index 65e6f2225..c97956e21 100644 --- a/bootloader/usb_data.cpp +++ b/bootloader/usb_data.cpp @@ -1,7 +1,7 @@ #include #include #include -#include +#include #include #include #include @@ -27,13 +27,13 @@ const char * Bootloader::USBData::buildStringDescriptor(StringHeader header, uin } const Bootloader::USBData Bootloader::USBData::DEFAULT() { - return USBData("@Flash/0x90000000/08*004Kg,01*032Kg,63*064Kg,64*064Kg", Messages::upsilonBootloader, DFUData()); + return USBData("@Flash/0x90000000/08*004Kg,01*032Kg,63*064Kg,64*064Kg", Messages::upsilonBootloader, ProtectionState()); } const Bootloader::USBData Bootloader::USBData::BOOTLOADER_UPDATE() { - return USBData("@Flash/0x08000000/04*016Kg", Messages::bootloaderUpdate, DFUData(true, false)); + return USBData("@Flash/0x08000000/04*016Kg", Messages::bootloaderUpdate, ProtectionState(true, false)); } Bootloader::USBData Bootloader::USBData::Recovery(uint32_t startAddress, uint32_t size) { - return USBData(buildStringDescriptor(StringHeader::SRAM(), startAddress, size), Messages::upsilonRecovery, DFUData(false, false)); + return USBData(buildStringDescriptor(StringHeader::SRAM(), startAddress, size), Messages::upsilonRecovery, ProtectionState(false, false)); } \ No newline at end of file diff --git a/bootloader/usb_data.h b/bootloader/usb_data.h index 8cca7475f..aea3f16ec 100644 --- a/bootloader/usb_data.h +++ b/bootloader/usb_data.h @@ -6,9 +6,9 @@ namespace Bootloader { -class DFUData { +class ProtectionState { public: - DFUData(bool unlockInternal = false, bool unlockExternal = true) : m_protectInternal(!unlockInternal), m_protectExternal(!unlockExternal) {}; + ProtectionState(bool unlockInternal = false, bool unlockExternal = true) : m_protectInternal(!unlockInternal), m_protectExternal(!unlockExternal) {}; bool isProtectedInternal() const { return m_protectInternal; } bool isProtectedExternal() const { return m_protectExternal; } @@ -33,11 +33,11 @@ class USBData { const char * m_string; }; - USBData(const char * desc, const char * name, DFUData data = DFUData()) : m_stringDescriptor(desc), m_name(name), m_data(&data) {}; + USBData(const char * desc, const char * name, ProtectionState data = ProtectionState()) : m_stringDescriptor(desc), m_name(name), m_data(&data) {}; const char * stringDescriptor() const { return m_stringDescriptor; } const char * getName() const { return m_name; } - DFUData * getData() const { return m_data; } + ProtectionState * getData() const { return m_data; } static const char * buildStringDescriptor(StringHeader header, uint32_t startAddress, uint32_t size); @@ -48,7 +48,7 @@ class USBData { private: const char * m_stringDescriptor; const char * m_name; - DFUData * m_data; + ProtectionState * m_data; }; } diff --git a/bootloader/userland_header.cpp b/bootloader/userland_header.cpp index a86a62ce8..ee1f8b15d 100644 --- a/bootloader/userland_header.cpp +++ b/bootloader/userland_header.cpp @@ -14,7 +14,7 @@ const bool UserlandHeader::isValid() const { } const bool UserlandHeader::isOmega() const { - return m_ohm_header == OmegaMagic && m_ohm_footer == OmegaMagic; + return m_omegaMagicHeader == OmegaMagic && m_omegaMagicFooter == OmegaMagic; } @@ -23,7 +23,7 @@ const char * UserlandHeader::omegaVersion() const { } const bool UserlandHeader::isUpsilon() const { - return m_ups_header == UpsilonMagic && m_ups_footer == UpsilonMagic; + return m_upsilonMagicHeader == UpsilonMagic && m_upsilonMagicHeader == UpsilonMagic; } const char * UserlandHeader::upsilonVersion() const { diff --git a/bootloader/userland_header.h b/bootloader/userland_header.h index 27359b176..39356f3d3 100644 --- a/bootloader/userland_header.h +++ b/bootloader/userland_header.h @@ -34,14 +34,14 @@ private: uint32_t m_externalAppsRAMStart; uint32_t m_externalAppsRAMEnd; uint32_t m_footer; - uint32_t m_ohm_header; + uint32_t m_omegaMagicHeader; const char m_omegaVersion[16]; const volatile char m_username[16]; - uint32_t m_ohm_footer; - uint32_t m_ups_header; + uint32_t m_omegaMagicFooter; + uint32_t m_upsilonMagicHeader; const char m_UpsilonVersion[16]; uint32_t m_osType; - uint32_t m_ups_footer; + uint32_t m_upsilonMagicFooter; }; extern const UserlandHeader* s_userlandHeaderA; diff --git a/bootloader/utility.cpp b/bootloader/utility.cpp new file mode 100644 index 000000000..1329351e5 --- /dev/null +++ b/bootloader/utility.cpp @@ -0,0 +1,10 @@ +#include +#include + +int Bootloader::Utility::versionSum(const char * version, int length) { + int sum = 0; + for (int i = 0; i < length; i++) { + sum += version[i] * (strlen(version) * 100 - i * 10); + } + return sum; +} diff --git a/bootloader/itoa.h b/bootloader/utility.h similarity index 71% rename from bootloader/itoa.h rename to bootloader/utility.h index 1a8602528..f0739f35a 100644 --- a/bootloader/itoa.h +++ b/bootloader/utility.h @@ -5,7 +5,8 @@ namespace Bootloader { class Utility { public: static char * itoa(int value, char * result, int base); + static int versionSum(const char * version, int length); }; } -#endif // _BOOTLOADER_ITOA_H_ \ No newline at end of file +#endif diff --git a/ion/src/device/bootloader/usb/calculator.h b/ion/src/device/bootloader/usb/calculator.h index e4954edae..68e967cf8 100644 --- a/ion/src/device/bootloader/usb/calculator.h +++ b/ion/src/device/bootloader/usb/calculator.h @@ -161,7 +161,7 @@ private: ExtendedCompatIDDescriptor m_extendedCompatIdDescriptor; Descriptor * m_descriptors[8]; - /* m_descriptors contains only descriptors that sould be returned via the + /* m_descriptors contains only descriptors that should be returned via the * method descriptor(uint8_t type, uint8_t index), so do not count descriptors * included in other descriptors or returned by other functions. */ diff --git a/ion/src/device/bootloader/usb/dfu_interface.cpp b/ion/src/device/bootloader/usb/dfu_interface.cpp index 18562aef3..0b6abb611 100644 --- a/ion/src/device/bootloader/usb/dfu_interface.cpp +++ b/ion/src/device/bootloader/usb/dfu_interface.cpp @@ -211,7 +211,7 @@ void DFUInterface::eraseMemoryIfNeeded() { willErase(); - Bootloader::DFUData * config = getDfuConfig(); + Bootloader::ProtectionState * config = getDfuConfig(); if (config != nullptr) { // More simple to read @@ -242,7 +242,7 @@ void DFUInterface::writeOnMemory() { memcpy((void *)m_writeAddress, m_largeBuffer, m_largeBufferLength); } else if (Flash::SectorAtAddress(m_writeAddress) >= 0) { - Bootloader::DFUData * config = getDfuConfig(); + Bootloader::ProtectionState * config = getDfuConfig(); if (config != nullptr) { if (m_writeAddress >= 0x08000000 && m_writeAddress <= 0x08010000 && !m_dfuData.isProtectedInternal()) { @@ -306,7 +306,7 @@ void DFUInterface::leaveDFUAndReset() { } void DFUInterface::copyDfuData() { - m_dfuData = Bootloader::DFUData(!m_dfuConfig->isProtectedInternal(), !m_dfuConfig->isProtectedExternal()); + m_dfuData = Bootloader::ProtectionState(!m_dfuConfig->isProtectedInternal(), !m_dfuConfig->isProtectedExternal()); } } diff --git a/ion/src/device/bootloader/usb/dfu_interface.h b/ion/src/device/bootloader/usb/dfu_interface.h index 5602002ca..2cb2eaeee 100644 --- a/ion/src/device/bootloader/usb/dfu_interface.h +++ b/ion/src/device/bootloader/usb/dfu_interface.h @@ -40,8 +40,8 @@ public: void wholeDataSentCallback(SetupPacket * request, uint8_t * transferBuffer, uint16_t * transferBufferLength) override; bool isErasingAndWriting() const { return m_isErasingAndWriting; } - void setDfuConfig(Bootloader::DFUData * data) { m_dfuConfig = data; copyDfuData(); } - Bootloader::DFUData * getDfuConfig() const { return m_dfuConfig; } + void setDfuConfig(Bootloader::ProtectionState * data) { m_dfuConfig = data; copyDfuData(); } + Bootloader::ProtectionState * getDfuConfig() const { return m_dfuConfig; } protected: void setActiveInterfaceAlternative(uint8_t interfaceAlternativeIndex) override { @@ -180,9 +180,9 @@ private: uint32_t m_writeAddress; uint8_t m_bInterfaceAlternateSetting; bool m_isErasingAndWriting; - Bootloader::DFUData * m_dfuConfig; + Bootloader::ProtectionState * m_dfuConfig; uint32_t m_eraseAddress; - Bootloader::DFUData m_dfuData; + Bootloader::ProtectionState m_dfuData; }; } diff --git a/ion/src/device/bootloader/usb/stack/descriptor/device_capability_descriptor.h b/ion/src/device/bootloader/usb/stack/descriptor/device_capability_descriptor.h index f434352f2..8b67cabc6 100644 --- a/ion/src/device/bootloader/usb/stack/descriptor/device_capability_descriptor.h +++ b/ion/src/device/bootloader/usb/stack/descriptor/device_capability_descriptor.h @@ -1,5 +1,5 @@ -#ifndef ION_DEVICE_SHARED_USB_STACK_DEVICE_CAPABLITY_DESCRIPTOR_H -#define ION_DEVICE_SHARED_USB_STACK_DEVICE_CAPABLITY_DESCRIPTOR_H +#ifndef ION_DEVICE_SHARED_USB_STACK_DEVICE_CAPABILITY_DESCRIPTOR_H +#define ION_DEVICE_SHARED_USB_STACK_DEVICE_CAPABILITY_DESCRIPTOR_H #include "descriptor.h" diff --git a/ion/src/device/bootloader/usb/stack/descriptor/platform_device_capability_descriptor.h b/ion/src/device/bootloader/usb/stack/descriptor/platform_device_capability_descriptor.h index d2c425d4a..3c117348c 100644 --- a/ion/src/device/bootloader/usb/stack/descriptor/platform_device_capability_descriptor.h +++ b/ion/src/device/bootloader/usb/stack/descriptor/platform_device_capability_descriptor.h @@ -1,5 +1,5 @@ -#ifndef ION_DEVICE_SHARED_USB_STACK_PLATFORM_DEVICE_CAPABLITY_DESCRIPTOR_H -#define ION_DEVICE_SHARED_USB_STACK_PLATFORM_DEVICE_CAPABLITY_DESCRIPTOR_H +#ifndef ION_DEVICE_SHARED_USB_STACK_PLATFORM_DEVICE_CAPABILITY_DESCRIPTOR_H +#define ION_DEVICE_SHARED_USB_STACK_PLATFORM_DEVICE_CAPABILITY_DESCRIPTOR_H #include "device_capability_descriptor.h" diff --git a/ion/src/device/n0110/drivers/board.cpp b/ion/src/device/n0110/drivers/board.cpp index 23579c5a2..c133d492b 100644 --- a/ion/src/device/n0110/drivers/board.cpp +++ b/ion/src/device/n0110/drivers/board.cpp @@ -104,7 +104,7 @@ void initMPU() { * then an AHB error is given (AN4760). To prevent this to happen, we * configure the MPU to define the whole Quad-SPI addressable space as * strongly ordered, non-executable and not accessible. Plus, we define the - * Quad-SPI region corresponding to the Expternal Chip as executable and + * Quad-SPI region corresponding to the External Chip as executable and * fully accessible (AN4861). */ MPU.RNR()->setREGION(sector++); MPU.RBAR()->setADDR(0x90000000); diff --git a/ion/src/device/shared/drivers/display.cpp b/ion/src/device/shared/drivers/display.cpp index 508242dde..6cc1f762c 100644 --- a/ion/src/device/shared/drivers/display.cpp +++ b/ion/src/device/shared/drivers/display.cpp @@ -51,7 +51,7 @@ bool waitForVBlank() { uint64_t startTime = Timing::millis(); uint64_t timeout = startTime + timeoutDelta; - /* If current time is big enough, currentTime + timeout wraps aroud the + /* If current time is big enough, currentTime + timeout wraps around the * uint64_t. We need to take this into account when computing the terminating * event. * diff --git a/ion/src/device/shared/drivers/reset.cpp b/ion/src/device/shared/drivers/reset.cpp index ef6053570..67e52f3d6 100644 --- a/ion/src/device/shared/drivers/reset.cpp +++ b/ion/src/device/shared/drivers/reset.cpp @@ -49,7 +49,7 @@ void jump(uint32_t jumpIsrVectorAddress) { // Disable cache before reset Ion::Device::Cache::disable(); - /* Shutdown all clocks and periherals to mimic a hardware reset. */ + /* Shutdown all clocks and peripherals to mimic a hardware reset. */ Board::shutdownPeripherals(); internalFlashJump(jumpIsrVectorAddress); diff --git a/ion/src/device/shared/drivers/wakeup.h b/ion/src/device/shared/drivers/wakeup.h index d8cb22c7b..7bf12d585 100644 --- a/ion/src/device/shared/drivers/wakeup.h +++ b/ion/src/device/shared/drivers/wakeup.h @@ -8,7 +8,7 @@ namespace Device { namespace WakeUp { /* All wakeup functions can be called together without overwriting the same - * register. All togethed, they will set SYSCFG and EXTi registers as follow: + * register. All together, they will set SYSCFG and EXTi registers as follow: * * GPIO Pin Number|EXTI_EMR|EXTI_FTSR|EXTI_RTSR|EXTICR1|EXTICR2|EXTICR3| Wake up * ---------------+--------+---------+---------+-------+-------+-------+------------------------- diff --git a/ion/src/device/shared/regs/tim.h b/ion/src/device/shared/regs/tim.h index f92d119e6..704bd4fa9 100644 --- a/ion/src/device/shared/regs/tim.h +++ b/ion/src/device/shared/regs/tim.h @@ -17,7 +17,7 @@ public: }; class CCMR : Register64 { - /* We're declaring CCMR as a 64 bits register. CCMR doesn't exsist per se, + /* We're declaring CCMR as a 64 bits register. CCMR doesn't exist per se, * it is in fact the consolidation of CCMR1 and CCMR2. Both are 16 bits * registers, so one could expect the consolidation to be 32 bits. However, * both CCMR1 and CCMR2 live on 32-bits boundaries, so the consolidation has diff --git a/ion/src/device/shared/usb/calculator.h b/ion/src/device/shared/usb/calculator.h index 9c8b869fb..308beb66b 100644 --- a/ion/src/device/shared/usb/calculator.h +++ b/ion/src/device/shared/usb/calculator.h @@ -155,7 +155,7 @@ private: ExtendedCompatIDDescriptor m_extendedCompatIdDescriptor; Descriptor * m_descriptors[8]; - /* m_descriptors contains only descriptors that sould be returned via the + /* m_descriptors contains only descriptors that should be returned via the * method descriptor(uint8_t type, uint8_t index), so do not count descriptors * included in other descriptors or returned by other functions. */ diff --git a/ion/src/device/shared/usb/stack/descriptor/device_capability_descriptor.h b/ion/src/device/shared/usb/stack/descriptor/device_capability_descriptor.h index f434352f2..8b67cabc6 100644 --- a/ion/src/device/shared/usb/stack/descriptor/device_capability_descriptor.h +++ b/ion/src/device/shared/usb/stack/descriptor/device_capability_descriptor.h @@ -1,5 +1,5 @@ -#ifndef ION_DEVICE_SHARED_USB_STACK_DEVICE_CAPABLITY_DESCRIPTOR_H -#define ION_DEVICE_SHARED_USB_STACK_DEVICE_CAPABLITY_DESCRIPTOR_H +#ifndef ION_DEVICE_SHARED_USB_STACK_DEVICE_CAPABILITY_DESCRIPTOR_H +#define ION_DEVICE_SHARED_USB_STACK_DEVICE_CAPABILITY_DESCRIPTOR_H #include "descriptor.h" diff --git a/ion/src/device/shared/usb/stack/descriptor/platform_device_capability_descriptor.h b/ion/src/device/shared/usb/stack/descriptor/platform_device_capability_descriptor.h index d2c425d4a..3c117348c 100644 --- a/ion/src/device/shared/usb/stack/descriptor/platform_device_capability_descriptor.h +++ b/ion/src/device/shared/usb/stack/descriptor/platform_device_capability_descriptor.h @@ -1,5 +1,5 @@ -#ifndef ION_DEVICE_SHARED_USB_STACK_PLATFORM_DEVICE_CAPABLITY_DESCRIPTOR_H -#define ION_DEVICE_SHARED_USB_STACK_PLATFORM_DEVICE_CAPABLITY_DESCRIPTOR_H +#ifndef ION_DEVICE_SHARED_USB_STACK_PLATFORM_DEVICE_CAPABILITY_DESCRIPTOR_H +#define ION_DEVICE_SHARED_USB_STACK_PLATFORM_DEVICE_CAPABILITY_DESCRIPTOR_H #include "device_capability_descriptor.h"