From e2016d57de8ac1eb32b2da68ccaf4beaf0d54aa3 Mon Sep 17 00:00:00 2001 From: cousteau Date: Thu, 3 Mar 2016 23:10:26 +0100 Subject: [PATCH] Check for valid pin properly (fixes #4605) Fix for `pinMode()`, `digitalWrite()`, `digitalRead()` (issue #4605). Current behavior: look up pin number in `digital_pin_to_port_PGM[]` and then check if it returned `NOT_A_PIN`. Causes undefined behavior if provided `pin` number is out of the range of `digital_pin_to_port_PGM[]`. Proposed behavior (from issue #4605): check if `pin` is within the valid range of `digital_pin_to_port_PGM[]`, and THEN look it up. Additionally, remove second check for `port` not being `NOT_A_PIN` (which was useful for boards where the pin numbering skips some numbers). This can still be achieved by making `bit = digitalPinToBitMask(pin)` be 0 for invalid pins, which causes further bitwise operations such as `*reg &= ~bit;` and `*out |= bit;` to not actually modify the value of the register. (This removal makes the operation complete a bit faster for valid pins and slower for invalid pins, which I think is a good trade; plus it saves binary size.) --- cores/arduino/wiring_digital.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/cores/arduino/wiring_digital.c b/cores/arduino/wiring_digital.c index 27a62fc..c808cb8 100644 --- a/cores/arduino/wiring_digital.c +++ b/cores/arduino/wiring_digital.c @@ -26,14 +26,16 @@ #include "wiring_private.h" #include "pins_arduino.h" +#define PIN_MAX (sizeof digital_pin_to_port_PGM / sizeof *digital_pin_to_port_PGM) + void pinMode(uint8_t pin, uint8_t mode) { - uint8_t bit = digitalPinToBitMask(pin); - uint8_t port = digitalPinToPort(pin); + if (pin >= PIN_MAX) return; + + uint8_t bit = digitalPinToBitMask(pin); // bit mask, or 0 for invalid pins + uint8_t port = digitalPinToPort(pin); // must be a valid port (even for invalid pins) volatile uint8_t *reg, *out; - if (port == NOT_A_PIN) return; - // JWS: can I let the optimizer do this? reg = portModeRegister(port); out = portOutputRegister(port); @@ -137,13 +139,13 @@ static void turnOffPWM(uint8_t timer) void digitalWrite(uint8_t pin, uint8_t val) { + if (pin >= PIN_MAX) return; + uint8_t timer = digitalPinToTimer(pin); uint8_t bit = digitalPinToBitMask(pin); uint8_t port = digitalPinToPort(pin); volatile uint8_t *out; - if (port == NOT_A_PIN) return; - // If the pin that support PWM output, we need to turn it off // before doing a digital write. if (timer != NOT_ON_TIMER) turnOffPWM(timer); @@ -164,12 +166,12 @@ void digitalWrite(uint8_t pin, uint8_t val) int digitalRead(uint8_t pin) { + if (pin >= PIN_MAX) return LOW; + uint8_t timer = digitalPinToTimer(pin); uint8_t bit = digitalPinToBitMask(pin); uint8_t port = digitalPinToPort(pin); - if (port == NOT_A_PIN) return LOW; - // If the pin that support PWM output, we need to turn it off // before getting a digital reading. if (timer != NOT_ON_TIMER) turnOffPWM(timer);