[Reviews] Code correction and improvement

bootloader/Makefile

@@ -12,6 +12,7 @@ bootloader_src += $(addprefix bootloader/,\
   recovery.cpp \
   usb_data.cpp \
   itoa.cpp \
+  utility.cpp \
 )
 
 bootloader_images = $(addprefix bootloader/, \

bootloader/boot.cpp

@@ -7,6 +7,7 @@
 #include <bootloader/recovery.h>
 #include <bootloader/usb_data.h>
 #include <ion/src/device/shared/drivers/flash.h>
+#include <bootloader/utility.h>
 
 #include <assert.h>
 
@@ -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;
+    int vsum = Bootloader::Utility::versionSum(version, strlen(version));
-    for (int i = 0; i < strlen(version); i++) {
+    int minsum = Bootloader::Utility::versionSum(min, strlen(min));
-      vsum += version[i] * (100-i*15);
-    }
-    int minsum = 0;
-    for (int i = 0; i < strlen(min); i++) {
-      minsum += min[i] * (100-i*15);
-    }
     if (vsum >= minsum) {
       Interface::drawEpsilonAdvertisement();
       uint64_t scan = 0;

bootloader/itoa.cpp

@@ -1,6 +1,6 @@
 #include <stdio.h>
 #include <stdlib.h>
-#include <bootloader/itoa.h>
+#include <bootloader/utility.h>
 
 // https://www.techiedelight.com/implement-itoa-function-in-c/
 

bootloader/kernel_header.cpp

@@ -1,4 +1,5 @@
 #include <bootloader/kernel_header.h>
+#include <bootloader/utility.h>
 
 namespace Bootloader {
 
@@ -22,16 +23,10 @@ const void(*KernelHeader::startPointer() const)() {
   return m_startPointer;
 }
 
-const bool KernelHeader::isNewVersion() const {
+const bool KernelHeader::isAboveVersion16 () const {
-  int sum = 0;
+  int sum = Bootloader::Utility::versionSum(m_version, 2);
-  for (int i = 0; i < 2; i++) {
-    sum += m_version[i] * (5 - i);
-  }
   char newVersion[] = "16";
-  int min = 0;
+  int min = Bootloader::Utility::versionSum(newVersion, 2);
-  for (int i = 0; i < 2; i++) {
-    min += newVersion[i] * (5 - i);
-  }
   return sum >= min;
 }
 

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)();

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();

bootloader/usb_data.cpp

@@ -1,7 +1,7 @@
 #include <bootloader/usb_data.h>
 #include <stdint.h>
 #include <stddef.h>
-#include <bootloader/itoa.h>
+#include <bootloader/utility.h>
 #include <stdlib.h>
 #include <string.h>
 #include <bootloader/messages.h>
@@ -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));
 }

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;
 
 };
 }

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 {

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_omegaMagicFooter;
-  uint32_t m_ups_header;
+  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;

bootloader/utility.cpp

@@ -0,0 +1,10 @@
+#include <bootloader/utility.h>
+#include <string.h>
+
+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;
+}

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_
+#endif

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. */
 

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());
 }
 
 }

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(); }
+  void setDfuConfig(Bootloader::ProtectionState * data) { m_dfuConfig = data; copyDfuData(); }
-  Bootloader::DFUData * getDfuConfig() const { return m_dfuConfig; }
+  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;
 };
 
 }

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
+#ifndef ION_DEVICE_SHARED_USB_STACK_DEVICE_CAPABILITY_DESCRIPTOR_H
-#define ION_DEVICE_SHARED_USB_STACK_DEVICE_CAPABLITY_DESCRIPTOR_H
+#define ION_DEVICE_SHARED_USB_STACK_DEVICE_CAPABILITY_DESCRIPTOR_H
 
 #include "descriptor.h"
 

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
+#ifndef ION_DEVICE_SHARED_USB_STACK_PLATFORM_DEVICE_CAPABILITY_DESCRIPTOR_H
-#define ION_DEVICE_SHARED_USB_STACK_PLATFORM_DEVICE_CAPABLITY_DESCRIPTOR_H
+#define ION_DEVICE_SHARED_USB_STACK_PLATFORM_DEVICE_CAPABILITY_DESCRIPTOR_H
 
 #include "device_capability_descriptor.h"
 

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);

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.
    *

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);
 

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
  * ---------------+--------+---------+---------+-------+-------+-------+-------------------------

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

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. */
 

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
+#ifndef ION_DEVICE_SHARED_USB_STACK_DEVICE_CAPABILITY_DESCRIPTOR_H
-#define ION_DEVICE_SHARED_USB_STACK_DEVICE_CAPABLITY_DESCRIPTOR_H
+#define ION_DEVICE_SHARED_USB_STACK_DEVICE_CAPABILITY_DESCRIPTOR_H
 
 #include "descriptor.h"
 

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
+#ifndef ION_DEVICE_SHARED_USB_STACK_PLATFORM_DEVICE_CAPABILITY_DESCRIPTOR_H
-#define ION_DEVICE_SHARED_USB_STACK_PLATFORM_DEVICE_CAPABLITY_DESCRIPTOR_H
+#define ION_DEVICE_SHARED_USB_STACK_PLATFORM_DEVICE_CAPABILITY_DESCRIPTOR_H
 
 #include "device_capability_descriptor.h"