Merge pull request #1402 from keyboardio/f/eeprom-bullet-proofing

Additional fixes for possibly bad EEPROM data
keyboardio · Mar 5, 2024 · 6656217 · 6656217
2 parents d17e2b8 + 56dbb2d
commit 6656217
Show file tree

Hide file tree

Showing 6 changed files with 27 additions and 64 deletions.
diff --git a/plugins/Kaleidoscope-DynamicMacros/src/kaleidoscope/plugin/DynamicMacros.cpp b/plugins/Kaleidoscope-DynamicMacros/src/kaleidoscope/plugin/DynamicMacros.cpp
@@ -41,7 +41,7 @@ uint8_t DynamicMacros::updateDynamicMacroCache() {
 
   map_[0] = 0;
 
-  while (pos < storage_base_ + storage_size_) {
+  while (pos < storage_base_ + storage_size_ && current_id < MAX_MACRO_COUNT_) {
     macro = Runtime.storage().read(pos++);
     switch (macro) {
     case MACRO_ACTION_STEP_EXPLICIT_REPORT:
@@ -76,7 +76,7 @@ uint8_t DynamicMacros::updateDynamicMacroCache() {
       uint8_t keyCode, flags;
       do {
         keyCode = Runtime.storage().read(pos++);
-      } while (keyCode != 0);
+      } while ((pos < (storage_base_ + storage_size_)) && keyCode != 0);
       break;
     }
 
@@ -170,7 +170,7 @@ void DynamicMacros::play(uint8_t macro_id) {
       while (true) {
         key.setFlags(isKeycodeSequence ? 0 : storage.read(pos++));
         key.setKeyCode(storage.read(pos++));
-        if (key == Key_NoKey)
+        if (key == Key_NoKey || pos >= storage_base_ + storage_size_)
           break;
         tap(key);
         delay(interval);

diff --git a/plugins/Kaleidoscope-DynamicMacros/src/kaleidoscope/plugin/DynamicMacros.h b/plugins/Kaleidoscope-DynamicMacros/src/kaleidoscope/plugin/DynamicMacros.h
@@ -48,9 +48,10 @@ class DynamicMacros : public kaleidoscope::Plugin {
   void play(uint8_t seq_id);
 
  private:
+  static const uint8_t MAX_MACRO_COUNT_ = 32;
   uint16_t storage_base_;
   uint16_t storage_size_;
-  uint16_t map_[32];
+  uint16_t map_[MAX_MACRO_COUNT_];
   uint8_t macro_count_;
   uint8_t updateDynamicMacroCache();
 

diff --git a/plugins/Kaleidoscope-EEPROM-Settings/src/kaleidoscope/plugin/EEPROM-Settings.cpp b/plugins/Kaleidoscope-EEPROM-Settings/src/kaleidoscope/plugin/EEPROM-Settings.cpp
@@ -188,13 +188,6 @@ bool EEPROMSettings::isSliceValid(uint16_t start, size_t size) {
 
 /** Focus **/
 EventHandlerResult FocusSettingsCommand::onFocusEvent(const char *input) {
-  enum {
-    DEFAULT_LAYER,
-    IS_VALID,
-    GET_VERSION,
-    GET_CRC,
-  } sub_command;
-
   const char *cmd_defaultLayer = PSTR("settings.defaultLayer");
   const char *cmd_isValid      = PSTR("settings.valid?");
   const char *cmd_version      = PSTR("settings.version");
@@ -203,67 +196,36 @@ EventHandlerResult FocusSettingsCommand::onFocusEvent(const char *input) {
   if (::Focus.inputMatchesHelp(input))
     return ::Focus.printHelp(cmd_defaultLayer, cmd_isValid, cmd_version, cmd_crc);
 
-  if (::Focus.inputMatchesCommand(input, cmd_defaultLayer))
-    sub_command = DEFAULT_LAYER;
-  else if (::Focus.inputMatchesCommand(input, cmd_isValid))
-    sub_command = IS_VALID;
-  else if (::Focus.inputMatchesCommand(input, cmd_version))
-    sub_command = GET_VERSION;
-  else if (::Focus.inputMatchesCommand(input, cmd_crc))
-    sub_command = GET_CRC;
-  else
-    return EventHandlerResult::OK;
-
-  switch (sub_command) {
-  case DEFAULT_LAYER: {
+  if (::Focus.inputMatchesCommand(input, cmd_defaultLayer)) {
     if (::Focus.isEOL()) {
       ::Focus.send(::EEPROMSettings.default_layer());
     } else {
       uint8_t layer;
       ::Focus.read(layer);
       ::EEPROMSettings.default_layer(layer);
     }
-    break;
-  }
-  case IS_VALID:
+  } else if (::Focus.inputMatchesCommand(input, cmd_isValid)) {
     ::Focus.send(::EEPROMSettings.isValid());
-    break;
-  case GET_VERSION:
+  } else if (::Focus.inputMatchesCommand(input, cmd_version)) {
     ::Focus.send(::EEPROMSettings.version());
-    break;
-  case GET_CRC:
+  } else if (::Focus.inputMatchesCommand(input, cmd_crc)) {
     ::Focus.sendRaw(::CRCCalculator.crc, F("/"), ::EEPROMSettings.crc());
-    break;
+  } else {
+    return EventHandlerResult::OK;
   }
 
   return EventHandlerResult::EVENT_CONSUMED;
 }
 
 EventHandlerResult FocusEEPROMCommand::onFocusEvent(const char *input) {
-  enum {
-    CONTENTS,
-    FREE,
-    ERASE,
-  } sub_command;
-
   const char *cmd_contents = PSTR("eeprom.contents");
   const char *cmd_free     = PSTR("eeprom.free");
   const char *cmd_erase    = PSTR("eeprom.erase");
 
   if (::Focus.inputMatchesHelp(input))
     return ::Focus.printHelp(cmd_contents, cmd_free, cmd_erase);
 
-  if (::Focus.inputMatchesCommand(input, cmd_contents))
-    sub_command = CONTENTS;
-  else if (::Focus.inputMatchesCommand(input, cmd_free))
-    sub_command = FREE;
-  else if (::Focus.inputMatchesCommand(input, cmd_erase))
-    sub_command = ERASE;
-  else
-    return EventHandlerResult::OK;
-
-  switch (sub_command) {
-  case CONTENTS: {
+  if (::Focus.inputMatchesCommand(input, cmd_contents)) {
     if (::Focus.isEOL()) {
       for (uint16_t i = 0; i < Runtime.storage().length(); i++) {
         uint8_t d = Runtime.storage().read(i);
@@ -277,21 +239,18 @@ EventHandlerResult FocusEEPROMCommand::onFocusEvent(const char *input) {
       }
       Runtime.storage().commit();
     }
-
-    break;
-  }
-  case FREE:
+  } else if (::Focus.inputMatchesCommand(input, cmd_free)) {
     ::Focus.send(Runtime.storage().length() - ::EEPROMSettings.used());
-    break;
-  case ERASE: {
+  } else if (::Focus.inputMatchesCommand(input, cmd_erase)) {
     for (uint16_t i = 0; i < Runtime.storage().length(); i++) {
       Runtime.storage().update(i, EEPROMSettings::EEPROM_UNINITIALIZED_BYTE);
     }
     Runtime.storage().commit();
     Runtime.device().rebootBootloader();
-    break;
-  }
+  } else {
+    return EventHandlerResult::OK;
   }
+
   return EventHandlerResult::EVENT_CONSUMED;
 }
 

diff --git a/plugins/Kaleidoscope-Escape-OneShot/src/kaleidoscope/plugin/Escape-OneShot-Config.cpp b/plugins/Kaleidoscope-Escape-OneShot/src/kaleidoscope/plugin/Escape-OneShot-Config.cpp
@@ -58,10 +58,10 @@ EventHandlerResult EscapeOneShotConfig::onFocusEvent(const char *input) {
     Key k;
     ::Focus.read(k);
     ::EscapeOneShot.setCancelKey(k);
+    Runtime.storage().put(settings_base_, ::EscapeOneShot.settings_);
+    Runtime.storage().commit();
   }
 
-  Runtime.storage().put(settings_base_, ::EscapeOneShot.settings_);
-  Runtime.storage().commit();
   return EventHandlerResult::EVENT_CONSUMED;
 }
 

diff --git a/plugins/Kaleidoscope-MouseKeys/src/kaleidoscope/plugin/MouseKeysConfig.cpp b/plugins/Kaleidoscope-MouseKeys/src/kaleidoscope/plugin/MouseKeysConfig.cpp
@@ -137,12 +137,13 @@ EventHandlerResult MouseKeysConfig::onFocusEvent(const char *input) {
       ::MouseKeys.setWarpGridSize(arg);
       break;
     }
+    // Update settings stored in EEPROM, and indicate that this Focus event has
+    // been handled successfully.
+    Runtime.storage().put(settings_base_, ::MouseKeys.settings_);
+    Runtime.storage().commit();
   }
 
-  // Update settings stored in EEPROM, and indicate that this Focus event has
-  // been handled successfully.
-  Runtime.storage().put(settings_base_, ::MouseKeys.settings_);
-  Runtime.storage().commit();
+
   return EventHandlerResult::EVENT_CONSUMED;
 }
 

diff --git a/plugins/Kaleidoscope-SpaceCadet/src/kaleidoscope/plugin/SpaceCadetConfig.cpp b/plugins/Kaleidoscope-SpaceCadet/src/kaleidoscope/plugin/SpaceCadetConfig.cpp
@@ -37,8 +37,10 @@ EventHandlerResult SpaceCadetConfig::onSetup() {
 }
 
 void SpaceCadetConfig::disableSpaceCadetIfUnconfigured() {
-  if (Runtime.storage().isSliceUninitialized(settings_base_, sizeof(SpaceCadet::settings_)))
+  if (Runtime.storage().isSliceUninitialized(settings_base_, sizeof(SpaceCadet::settings_)) ||
+      (::SpaceCadet.settings_.mode != SpaceCadet::Mode::ON && ::SpaceCadet.settings_.mode != SpaceCadet::Mode::NO_DELAY)) {
     ::SpaceCadet.disable();
+  }
 }
 
 EventHandlerResult SpaceCadetConfig::onFocusEvent(const char *input) {