From e7f9edba2d78a75e90747a00758915a055a7c654 Mon Sep 17 00:00:00 2001
From: Michael Richters <gedankenexperimenter@gmail.com>
Date: Sat, 11 Nov 2017 23:46:08 -0600
Subject: [PATCH 1/9] Added a check to prevent reading past the end of the
 keymaps[] array

This relys on the 'LayerCount' variable being correct and defined in
the firmware sketch file (along with 'keymaps')
---
 src/layers.cpp | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/layers.cpp b/src/layers.cpp
index 702672a8..88f43c80 100644
--- a/src/layers.cpp
+++ b/src/layers.cpp
@@ -125,6 +125,11 @@ void Layer_::move(uint8_t layer) {
 }
 
 void Layer_::on(uint8_t layer) {
+  if (layer >= LayerCount) {
+    // Trying to turn on a layer that doesn't exist; abort
+    return;
+  }
+
   bool wasOn = isOn(layer);
 
   bitSet(LayerState, layer);

From 9db5036a5cca460320d5a21459a32fc9a15c58b9 Mon Sep 17 00:00:00 2001
From: Michael Richters <gedankenexperimenter@gmail.com>
Date: Sun, 12 Nov 2017 00:05:39 -0600
Subject: [PATCH 2/9] Make `LayerCount` available in layers.cpp

---
 src/layers.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/layers.h b/src/layers.h
index 4b0ae236..d3f3b994 100644
--- a/src/layers.h
+++ b/src/layers.h
@@ -4,6 +4,9 @@
 #include "key_defs.h"
 #include KALEIDOSCOPE_HARDWARE_H
 
+// The total number of defined layers in the firmware sketch keymaps[] array
+extern const uint8_t LayerCount;
+
 class Layer_ {
  public:
   Layer_(void);

From 7ddc0249bbb764dbaece9cd39bbefb6f3a64b7ff Mon Sep 17 00:00:00 2001
From: Michael Richters <gedankenexperimenter@gmail.com>
Date: Sun, 12 Nov 2017 20:09:24 -0600
Subject: [PATCH 3/9] Added `LayerCount` variable to the example sketches

This took some trial and error to figure out, but once I determined
that the example sketches were being built, I made this change to keep
the build working. Hopefully this will satisfy Travis-CI.
---
 examples/AppSwitcher/AppSwitcher.ino   | 5 +++++
 examples/Kaleidoscope/Kaleidoscope.ino | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/examples/AppSwitcher/AppSwitcher.ino b/examples/AppSwitcher/AppSwitcher.ino
index e8b231d5..eb385cc4 100644
--- a/examples/AppSwitcher/AppSwitcher.ino
+++ b/examples/AppSwitcher/AppSwitcher.ino
@@ -46,6 +46,11 @@ const Key keymaps[][ROWS][COLS] PROGMEM = {
 };
 /* *INDENT-ON* */
 
+// Calculate the number of layers defined in the "keymaps[]" array
+// above for use elsewhere in Kaleidoscope. Don't remove or modify
+// this line; it's used to prevent reading past the end of the array!
+const uint8_t LayerCount PROGMEM = sizeof(keymaps) / sizeof(*keymaps);
+
 
 const macro_t *macroAction(uint8_t macroIndex, uint8_t keyState) {
   switch (macroIndex) {
diff --git a/examples/Kaleidoscope/Kaleidoscope.ino b/examples/Kaleidoscope/Kaleidoscope.ino
index 64c42bb7..b5f10424 100644
--- a/examples/Kaleidoscope/Kaleidoscope.ino
+++ b/examples/Kaleidoscope/Kaleidoscope.ino
@@ -65,6 +65,11 @@ const Key keymaps[][ROWS][COLS] PROGMEM = {
 
 };
 
+// Calculate the number of layers defined in the "keymaps[]" array
+// above for use elsewhere in Kaleidoscope. Don't remove or modify
+// this line; it's used to prevent reading past the end of the array!
+const uint8_t LayerCount PROGMEM = sizeof(keymaps) / sizeof(*keymaps);
+
 static kaleidoscope::LEDSolidColor solidRed(60, 0, 0);
 static kaleidoscope::LEDSolidColor solidOrange(60, 20, 0);
 static kaleidoscope::LEDSolidColor solidYellow(40, 35, 0);

From 50ac31d0f560b5f65745598846e6afb8434a68c6 Mon Sep 17 00:00:00 2001
From: Michael Richters <gedankenexperimenter@gmail.com>
Date: Mon, 13 Nov 2017 15:02:58 -0600
Subject: [PATCH 4/9] Added a "sketch-trailer.h" header file

This file is meant to be included in sketch files in order to make
data available to Kaleidoscope functions. In particular, the size of
the keymaps[] array (i.e. the number of defined layers), which is
needed in order to prevent reading uninitialized memory past the end
of that array due to Key_KeymapNext_Momentary.
---
 examples/AppSwitcher/AppSwitcher.ino   | 8 +++-----
 examples/Kaleidoscope/Kaleidoscope.ino | 8 +++-----
 src/sketch-trailer.h                   | 6 ++++++
 3 files changed, 12 insertions(+), 10 deletions(-)
 create mode 100644 src/sketch-trailer.h

diff --git a/examples/AppSwitcher/AppSwitcher.ino b/examples/AppSwitcher/AppSwitcher.ino
index eb385cc4..6b561632 100644
--- a/examples/AppSwitcher/AppSwitcher.ino
+++ b/examples/AppSwitcher/AppSwitcher.ino
@@ -46,11 +46,6 @@ const Key keymaps[][ROWS][COLS] PROGMEM = {
 };
 /* *INDENT-ON* */
 
-// Calculate the number of layers defined in the "keymaps[]" array
-// above for use elsewhere in Kaleidoscope. Don't remove or modify
-// this line; it's used to prevent reading past the end of the array!
-const uint8_t LayerCount PROGMEM = sizeof(keymaps) / sizeof(*keymaps);
-
 
 const macro_t *macroAction(uint8_t macroIndex, uint8_t keyState) {
   switch (macroIndex) {
@@ -71,3 +66,6 @@ void setup() {
 void loop() {
   Kaleidoscope.loop();
 }
+
+// This line should always be at the end of the sketch file
+#include <sketch-trailer.h>
diff --git a/examples/Kaleidoscope/Kaleidoscope.ino b/examples/Kaleidoscope/Kaleidoscope.ino
index b5f10424..eae88d7f 100644
--- a/examples/Kaleidoscope/Kaleidoscope.ino
+++ b/examples/Kaleidoscope/Kaleidoscope.ino
@@ -65,11 +65,6 @@ const Key keymaps[][ROWS][COLS] PROGMEM = {
 
 };
 
-// Calculate the number of layers defined in the "keymaps[]" array
-// above for use elsewhere in Kaleidoscope. Don't remove or modify
-// this line; it's used to prevent reading past the end of the array!
-const uint8_t LayerCount PROGMEM = sizeof(keymaps) / sizeof(*keymaps);
-
 static kaleidoscope::LEDSolidColor solidRed(60, 0, 0);
 static kaleidoscope::LEDSolidColor solidOrange(60, 20, 0);
 static kaleidoscope::LEDSolidColor solidYellow(40, 35, 0);
@@ -109,3 +104,6 @@ void setup() {
 void loop() {
   Kaleidoscope.loop();
 }
+
+// This line should always be at the end of the sketch file
+#include <sketch-trailer.h>
diff --git a/src/sketch-trailer.h b/src/sketch-trailer.h
new file mode 100644
index 00000000..9a928920
--- /dev/null
+++ b/src/sketch-trailer.h
@@ -0,0 +1,6 @@
+#pragma once
+
+// Calculate the number of layers defined in the "keymaps[]"
+// array. This needs to be included in the sketch after "keymaps" is
+// defined
+const uint8_t LayerCount PROGMEM = sizeof(keymaps) / sizeof(*keymaps);

From de39e20d7844c57a6199101785aef461135d5168 Mon Sep 17 00:00:00 2001
From: Michael Richters <gedankenexperimenter@gmail.com>
Date: Sat, 18 Nov 2017 18:13:15 -0600
Subject: [PATCH 5/9] Define keymap layers with CREATE_KEYMAP macro

This macro allows the definition of the LayerCount variable and the
keymaps[] array together. It shouldn't break old sketches, but this is
probably not all that's necessary; LayerCount still doesn't get
initialized outside the macro.
---
 examples/AppSwitcher/AppSwitcher.ino   | 7 ++-----
 examples/Kaleidoscope/Kaleidoscope.ino | 8 ++------
 src/layers.cpp                         | 2 +-
 src/layers.h                           | 5 +++++
 src/sketch-trailer.h                   | 6 ------
 5 files changed, 10 insertions(+), 18 deletions(-)
 delete mode 100644 src/sketch-trailer.h

diff --git a/examples/AppSwitcher/AppSwitcher.ino b/examples/AppSwitcher/AppSwitcher.ino
index 6b561632..aecb1242 100644
--- a/examples/AppSwitcher/AppSwitcher.ino
+++ b/examples/AppSwitcher/AppSwitcher.ino
@@ -24,7 +24,7 @@
 #include "Macros.h"
 
 /* *INDENT-OFF* */
-const Key keymaps[][ROWS][COLS] PROGMEM = {
+CREATE_KEYMAP(
   [0] = KEYMAP_STACKED
   (
    Key_NoKey,    Key_1, Key_2, Key_3, Key_4, Key_5, Key_NoKey,
@@ -43,7 +43,7 @@ const Key keymaps[][ROWS][COLS] PROGMEM = {
    Key_RightShift, Key_RightAlt, Key_Spacebar, Key_RightControl,
    M(M_APPCANCEL)
    ),
-};
+	      )
 /* *INDENT-ON* */
 
 
@@ -66,6 +66,3 @@ void setup() {
 void loop() {
   Kaleidoscope.loop();
 }
-
-// This line should always be at the end of the sketch file
-#include <sketch-trailer.h>
diff --git a/examples/Kaleidoscope/Kaleidoscope.ino b/examples/Kaleidoscope/Kaleidoscope.ino
index eae88d7f..1964e08d 100644
--- a/examples/Kaleidoscope/Kaleidoscope.ino
+++ b/examples/Kaleidoscope/Kaleidoscope.ino
@@ -57,14 +57,13 @@
                                               Key_KeymapNext_Momentary,         Key_KeymapNext_Momentary \
 )
 
-const Key keymaps[][ROWS][COLS] PROGMEM = {
+CREATE_KEYMAP(
   QWERTY,
   GENERIC_FN2,
   NUMPAD
+)
 
 
-};
-
 static kaleidoscope::LEDSolidColor solidRed(60, 0, 0);
 static kaleidoscope::LEDSolidColor solidOrange(60, 20, 0);
 static kaleidoscope::LEDSolidColor solidYellow(40, 35, 0);
@@ -104,6 +103,3 @@ void setup() {
 void loop() {
   Kaleidoscope.loop();
 }
-
-// This line should always be at the end of the sketch file
-#include <sketch-trailer.h>
diff --git a/src/layers.cpp b/src/layers.cpp
index 88f43c80..7bfe05ab 100644
--- a/src/layers.cpp
+++ b/src/layers.cpp
@@ -125,8 +125,8 @@ void Layer_::move(uint8_t layer) {
 }
 
 void Layer_::on(uint8_t layer) {
+  // If we're trying to turn on a layer that doesn't exist; abort
   if (layer >= LayerCount) {
-    // Trying to turn on a layer that doesn't exist; abort
     return;
   }
 
diff --git a/src/layers.h b/src/layers.h
index d3f3b994..19512477 100644
--- a/src/layers.h
+++ b/src/layers.h
@@ -4,9 +4,14 @@
 #include "key_defs.h"
 #include KALEIDOSCOPE_HARDWARE_H
 
+#define CREATE_KEYMAP(layers...)				\
+  const Key keymaps[][ROWS][COLS] PROGMEM = { layers };		\
+  const uint8_t LayerCount = sizeof(keymaps) / sizeof(*keymaps);
+
 // The total number of defined layers in the firmware sketch keymaps[] array
 extern const uint8_t LayerCount;
 
+
 class Layer_ {
  public:
   Layer_(void);
diff --git a/src/sketch-trailer.h b/src/sketch-trailer.h
deleted file mode 100644
index 9a928920..00000000
--- a/src/sketch-trailer.h
+++ /dev/null
@@ -1,6 +0,0 @@
-#pragma once
-
-// Calculate the number of layers defined in the "keymaps[]"
-// array. This needs to be included in the sketch after "keymaps" is
-// defined
-const uint8_t LayerCount PROGMEM = sizeof(keymaps) / sizeof(*keymaps);

From 2784d0e9a9ce94308ba323ea2e1ff218eaaa1285 Mon Sep 17 00:00:00 2001
From: Michael Richters <gedankenexperimenter@gmail.com>
Date: Sun, 19 Nov 2017 12:00:07 -0600
Subject: [PATCH 6/9] Use weak attribute declaration for LayerCount

Declaring LayerCount as a weak symbol in layers.cpp lets us override
it if the CREATE_KEYMAP macro is used to define the keymap in the
sketch file, but still allows old sketch files to compile without
errors.

Still some changes necessary to allow old sketches to work
properly (Layer.on() will abort before doing anything).
---
 src/layers.cpp | 3 +++
 src/layers.h   | 5 +----
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/layers.cpp b/src/layers.cpp
index 7bfe05ab..0cf9129d 100644
--- a/src/layers.cpp
+++ b/src/layers.cpp
@@ -8,6 +8,9 @@ Key Layer_::liveCompositeKeymap[ROWS][COLS];
 uint8_t Layer_::activeLayers[ROWS][COLS];
 Key(*Layer_::getKey)(uint8_t layer, byte row, byte col) = Layer.getKeyFromPROGMEM;
 
+// The total number of defined layers in the firmware sketch keymaps[] array
+uint8_t LayerCount __attribute__((weak)) = 0;
+
 static void handleKeymapKeyswitchEvent(Key keymapEntry, uint8_t keyState) {
   if (keymapEntry.keyCode >= LAYER_SHIFT_OFFSET) {
     uint8_t target = keymapEntry.keyCode - LAYER_SHIFT_OFFSET;
diff --git a/src/layers.h b/src/layers.h
index 19512477..f34ce0e4 100644
--- a/src/layers.h
+++ b/src/layers.h
@@ -6,10 +6,7 @@
 
 #define CREATE_KEYMAP(layers...)				\
   const Key keymaps[][ROWS][COLS] PROGMEM = { layers };		\
-  const uint8_t LayerCount = sizeof(keymaps) / sizeof(*keymaps);
-
-// The total number of defined layers in the firmware sketch keymaps[] array
-extern const uint8_t LayerCount;
+  uint8_t LayerCount = sizeof(keymaps) / sizeof(*keymaps);
 
 
 class Layer_ {

From 36b461a99a41835a636becc7760c54b26cd7228a Mon Sep 17 00:00:00 2001
From: Michael Richters <gedankenexperimenter@gmail.com>
Date: Sun, 19 Nov 2017 12:16:30 -0600
Subject: [PATCH 7/9] If using an old sketch, ignore LayerCount

If it's an old sketch, LayerCount will default to 0, so in order for
Layer.on() to function, don't bother checking for out-of-bounds if
LayerCount == 0.
---
 src/layers.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/layers.cpp b/src/layers.cpp
index 0cf9129d..125ca4a4 100644
--- a/src/layers.cpp
+++ b/src/layers.cpp
@@ -129,9 +129,8 @@ void Layer_::move(uint8_t layer) {
 
 void Layer_::on(uint8_t layer) {
   // If we're trying to turn on a layer that doesn't exist; abort
-  if (layer >= LayerCount) {
+  if (LayerCount != 0 && layer >= LayerCount)
     return;
-  }
 
   bool wasOn = isOn(layer);
 

From 0fb2abf6bc7a995c2a02505bf1aae67b3d40ea89 Mon Sep 17 00:00:00 2001
From: Michael Richters <gedankenexperimenter@gmail.com>
Date: Sun, 19 Nov 2017 12:44:32 -0600
Subject: [PATCH 8/9] Added better comments to CREATE_KEYMAP() & LayerCount
 declarations

Also improved comment in Layer.on() conditional
---
 src/layers.cpp | 7 +++++--
 src/layers.h   | 3 +++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/layers.cpp b/src/layers.cpp
index 125ca4a4..b3263d42 100644
--- a/src/layers.cpp
+++ b/src/layers.cpp
@@ -8,7 +8,9 @@ Key Layer_::liveCompositeKeymap[ROWS][COLS];
 uint8_t Layer_::activeLayers[ROWS][COLS];
 Key(*Layer_::getKey)(uint8_t layer, byte row, byte col) = Layer.getKeyFromPROGMEM;
 
-// The total number of defined layers in the firmware sketch keymaps[] array
+// The total number of defined layers in the firmware sketch keymaps[]
+// array. If the keymap wasn't defined using CREATE_KEYMAP() in the
+// sketch file, LayerCount gets the default value of zero.
 uint8_t LayerCount __attribute__((weak)) = 0;
 
 static void handleKeymapKeyswitchEvent(Key keymapEntry, uint8_t keyState) {
@@ -128,7 +130,8 @@ void Layer_::move(uint8_t layer) {
 }
 
 void Layer_::on(uint8_t layer) {
-  // If we're trying to turn on a layer that doesn't exist; abort
+  // If we're trying to turn on a layer that doesn't exist, abort (but
+  // if the keymap wasn't defined using CREATE_KEYMAP(), proceed anyway
   if (LayerCount != 0 && layer >= LayerCount)
     return;
 
diff --git a/src/layers.h b/src/layers.h
index f34ce0e4..09a2c6dc 100644
--- a/src/layers.h
+++ b/src/layers.h
@@ -4,6 +4,9 @@
 #include "key_defs.h"
 #include KALEIDOSCOPE_HARDWARE_H
 
+// Macro for defining the keymap. This should be used in the sketch
+// file (*.ino) to define the keymap[] array that holds the user's
+// layers. It also computes the number of layers in that keymap.
 #define CREATE_KEYMAP(layers...)				\
   const Key keymaps[][ROWS][COLS] PROGMEM = { layers };		\
   uint8_t LayerCount = sizeof(keymaps) / sizeof(*keymaps);

From 195d6bc413cfe372491e9fd0c67b41d7b2f8038c Mon Sep 17 00:00:00 2001
From: Michael Richters <gedankenexperimenter@gmail.com>
Date: Sat, 25 Nov 2017 23:00:13 -0600
Subject: [PATCH 9/9] Better compliance with coding style guide

* s/LayerCount/layer_count/
* s/CREATE_KEYMAP/KEYMAPS/
---
 examples/AppSwitcher/AppSwitcher.ino   | 4 ++--
 examples/Kaleidoscope/Kaleidoscope.ino | 2 +-
 src/layers.cpp                         | 6 +++---
 src/layers.h                           | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/examples/AppSwitcher/AppSwitcher.ino b/examples/AppSwitcher/AppSwitcher.ino
index aecb1242..7303cc33 100644
--- a/examples/AppSwitcher/AppSwitcher.ino
+++ b/examples/AppSwitcher/AppSwitcher.ino
@@ -24,7 +24,7 @@
 #include "Macros.h"
 
 /* *INDENT-OFF* */
-CREATE_KEYMAP(
+KEYMAPS(
   [0] = KEYMAP_STACKED
   (
    Key_NoKey,    Key_1, Key_2, Key_3, Key_4, Key_5, Key_NoKey,
@@ -43,7 +43,7 @@ CREATE_KEYMAP(
    Key_RightShift, Key_RightAlt, Key_Spacebar, Key_RightControl,
    M(M_APPCANCEL)
    ),
-	      )
+)
 /* *INDENT-ON* */
 
 
diff --git a/examples/Kaleidoscope/Kaleidoscope.ino b/examples/Kaleidoscope/Kaleidoscope.ino
index 1964e08d..e3a90086 100644
--- a/examples/Kaleidoscope/Kaleidoscope.ino
+++ b/examples/Kaleidoscope/Kaleidoscope.ino
@@ -57,7 +57,7 @@
                                               Key_KeymapNext_Momentary,         Key_KeymapNext_Momentary \
 )
 
-CREATE_KEYMAP(
+KEYMAPS(
   QWERTY,
   GENERIC_FN2,
   NUMPAD
diff --git a/src/layers.cpp b/src/layers.cpp
index b3263d42..b647510a 100644
--- a/src/layers.cpp
+++ b/src/layers.cpp
@@ -10,8 +10,8 @@ Key(*Layer_::getKey)(uint8_t layer, byte row, byte col) = Layer.getKeyFromPROGME
 
 // The total number of defined layers in the firmware sketch keymaps[]
 // array. If the keymap wasn't defined using CREATE_KEYMAP() in the
-// sketch file, LayerCount gets the default value of zero.
-uint8_t LayerCount __attribute__((weak)) = 0;
+// sketch file, layer_count gets the default value of zero.
+uint8_t layer_count __attribute__((weak)) = 0;
 
 static void handleKeymapKeyswitchEvent(Key keymapEntry, uint8_t keyState) {
   if (keymapEntry.keyCode >= LAYER_SHIFT_OFFSET) {
@@ -132,7 +132,7 @@ void Layer_::move(uint8_t layer) {
 void Layer_::on(uint8_t layer) {
   // If we're trying to turn on a layer that doesn't exist, abort (but
   // if the keymap wasn't defined using CREATE_KEYMAP(), proceed anyway
-  if (LayerCount != 0 && layer >= LayerCount)
+  if (layer_count != 0 && layer >= layer_count)
     return;
 
   bool wasOn = isOn(layer);
diff --git a/src/layers.h b/src/layers.h
index 09a2c6dc..d23868c1 100644
--- a/src/layers.h
+++ b/src/layers.h
@@ -7,9 +7,9 @@
 // Macro for defining the keymap. This should be used in the sketch
 // file (*.ino) to define the keymap[] array that holds the user's
 // layers. It also computes the number of layers in that keymap.
-#define CREATE_KEYMAP(layers...)				\
+#define KEYMAPS(layers...)				\
   const Key keymaps[][ROWS][COLS] PROGMEM = { layers };		\
-  uint8_t LayerCount = sizeof(keymaps) / sizeof(*keymaps);
+  uint8_t layer_count = sizeof(keymaps) / sizeof(*keymaps);
 
 
 class Layer_ {