From 51a7537d87fedf57efad83527f156d1d4d25c4ba Mon Sep 17 00:00:00 2001
From: Gergely Nagy <algernon@keyboard.io>
Date: Fri, 15 Feb 2019 08:17:13 +0100
Subject: [PATCH] OneShot: Fix isPressed() and isSticky()

I tried to be smart and use `sizeof()` instead of hard-coding the array size,
but I used it wrong, and we iterated to 32 instead of 16, overflowing the array
and looking at parts of the memory we had no business looking at. This resulted
in `isPressed()` and `isSticky()` always being true.

Instead of trying to be clever, use `OneShot::ONESHOT_KEY_COUNT` throughout,
which is a constexpr defined at 16, the number of oneshot keys we have.

Fixes #572.

Signed-off-by: Gergely Nagy <algernon@keyboard.io>
---
 src/kaleidoscope/plugin/OneShot.cpp | 16 ++++++++--------
 src/kaleidoscope/plugin/OneShot.h   |  5 +++--
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/src/kaleidoscope/plugin/OneShot.cpp b/src/kaleidoscope/plugin/OneShot.cpp
index 38ad85ef..d9485221 100644
--- a/src/kaleidoscope/plugin/OneShot.cpp
+++ b/src/kaleidoscope/plugin/OneShot.cpp
@@ -26,13 +26,13 @@ uint32_t OneShot::start_time_ = 0;
 uint16_t OneShot::time_out = 2500;
 uint16_t OneShot::hold_time_out = 250;
 int16_t OneShot::double_tap_time_out = -1;
-OneShot::key_state_t OneShot::state_[16];
+OneShot::key_state_t OneShot::state_[OneShot::ONESHOT_KEY_COUNT];
 Key OneShot::prev_key_;
 bool OneShot::should_cancel_ = false;
 bool OneShot::should_cancel_stickies_ = false;
 
 bool OneShot::isPressed() {
-  for (uint8_t i = 0; i < sizeof(state_); i++) {
+  for (uint8_t i = 0; i < ONESHOT_KEY_COUNT; i++) {
     if (state_[i].pressed)
       return true;
   }
@@ -40,7 +40,7 @@ bool OneShot::isPressed() {
 }
 
 bool OneShot::isSticky() {
-  for (uint8_t i = 0; i < sizeof(state_); i++) {
+  for (uint8_t i = 0; i < ONESHOT_KEY_COUNT; i++) {
     if (state_[i].sticky)
       return true;
   }
@@ -209,7 +209,7 @@ void OneShot::inject(Key mapped_key, uint8_t key_state) {
 // --- glue code ---
 
 bool OneShot::isActive(void) {
-  for (uint8_t i = 0; i < 16; i++) {
+  for (uint8_t i = 0; i < ONESHOT_KEY_COUNT; i++) {
     if ((state_[i].active && !hasTimedOut()) ||
         state_[i].pressed ||
         state_[i].sticky)
@@ -256,25 +256,25 @@ void OneShot::disableStickability(Key key) {
 }
 
 void OneShot::enableStickabilityForModifiers() {
-  for (uint8_t i = 0; i < 8; i++) {
+  for (uint8_t i = 0; i < ONESHOT_KEY_COUNT / 2; i++) {
     state_[i].stickable = true;
   }
 }
 
 void OneShot::enableStickabilityForLayers() {
-  for (uint8_t i = 8; i < 16; i++) {
+  for (uint8_t i = ONESHOT_KEY_COUNT / 2; i < ONESHOT_KEY_COUNT; i++) {
     state_[i].stickable = true;
   }
 }
 
 void OneShot::disableStickabilityForModifiers() {
-  for (uint8_t i = 0; i < 8; i++) {
+  for (uint8_t i = 0; i < ONESHOT_KEY_COUNT / 2; i++) {
     state_[i].stickable = false;
   }
 }
 
 void OneShot::disableStickabilityForLayers() {
-  for (uint8_t i = 8; i < 16; i++) {
+  for (uint8_t i = ONESHOT_KEY_COUNT / 2; i < ONESHOT_KEY_COUNT; i++) {
     state_[i].stickable = false;
   }
 }
diff --git a/src/kaleidoscope/plugin/OneShot.h b/src/kaleidoscope/plugin/OneShot.h
index b8f04c61..b344e072 100644
--- a/src/kaleidoscope/plugin/OneShot.h
+++ b/src/kaleidoscope/plugin/OneShot.h
@@ -29,7 +29,7 @@ namespace plugin {
 class OneShot : public kaleidoscope::Plugin {
  public:
   OneShot(void) {
-    for (uint8_t i = 0; i < 16; i++) {
+    for (uint8_t i = 0; i < ONESHOT_KEY_COUNT; i++) {
       state_[i].stickable = true;
     }
   }
@@ -79,6 +79,7 @@ class OneShot : public kaleidoscope::Plugin {
   void inject(Key mapped_key, uint8_t key_state);
 
  private:
+  static constexpr uint8_t ONESHOT_KEY_COUNT = 16;
   typedef struct {
     bool active: 1;
     bool pressed: 1;
@@ -87,7 +88,7 @@ class OneShot : public kaleidoscope::Plugin {
     uint8_t __reserved: 4;
     uint8_t position;
   } key_state_t;
-  static key_state_t state_[16];
+  static key_state_t state_[ONESHOT_KEY_COUNT];
 
   static uint32_t start_time_;
   static Key prev_key_;