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.)
This commit is contained in:
parent
b084848f2e
commit
e2016d57de
|
@ -26,13 +26,15 @@
|
||||||
#include "wiring_private.h"
|
#include "wiring_private.h"
|
||||||
#include "pins_arduino.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)
|
void pinMode(uint8_t pin, uint8_t mode)
|
||||||
{
|
{
|
||||||
uint8_t bit = digitalPinToBitMask(pin);
|
if (pin >= PIN_MAX) return;
|
||||||
uint8_t port = digitalPinToPort(pin);
|
|
||||||
volatile uint8_t *reg, *out;
|
|
||||||
|
|
||||||
if (port == NOT_A_PIN) 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;
|
||||||
|
|
||||||
// JWS: can I let the optimizer do this?
|
// JWS: can I let the optimizer do this?
|
||||||
reg = portModeRegister(port);
|
reg = portModeRegister(port);
|
||||||
|
@ -137,13 +139,13 @@ static void turnOffPWM(uint8_t timer)
|
||||||
|
|
||||||
void digitalWrite(uint8_t pin, uint8_t val)
|
void digitalWrite(uint8_t pin, uint8_t val)
|
||||||
{
|
{
|
||||||
|
if (pin >= PIN_MAX) return;
|
||||||
|
|
||||||
uint8_t timer = digitalPinToTimer(pin);
|
uint8_t timer = digitalPinToTimer(pin);
|
||||||
uint8_t bit = digitalPinToBitMask(pin);
|
uint8_t bit = digitalPinToBitMask(pin);
|
||||||
uint8_t port = digitalPinToPort(pin);
|
uint8_t port = digitalPinToPort(pin);
|
||||||
volatile uint8_t *out;
|
volatile uint8_t *out;
|
||||||
|
|
||||||
if (port == NOT_A_PIN) return;
|
|
||||||
|
|
||||||
// If the pin that support PWM output, we need to turn it off
|
// If the pin that support PWM output, we need to turn it off
|
||||||
// before doing a digital write.
|
// before doing a digital write.
|
||||||
if (timer != NOT_ON_TIMER) turnOffPWM(timer);
|
if (timer != NOT_ON_TIMER) turnOffPWM(timer);
|
||||||
|
@ -164,12 +166,12 @@ void digitalWrite(uint8_t pin, uint8_t val)
|
||||||
|
|
||||||
int digitalRead(uint8_t pin)
|
int digitalRead(uint8_t pin)
|
||||||
{
|
{
|
||||||
|
if (pin >= PIN_MAX) return LOW;
|
||||||
|
|
||||||
uint8_t timer = digitalPinToTimer(pin);
|
uint8_t timer = digitalPinToTimer(pin);
|
||||||
uint8_t bit = digitalPinToBitMask(pin);
|
uint8_t bit = digitalPinToBitMask(pin);
|
||||||
uint8_t port = digitalPinToPort(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
|
// If the pin that support PWM output, we need to turn it off
|
||||||
// before getting a digital reading.
|
// before getting a digital reading.
|
||||||
if (timer != NOT_ON_TIMER) turnOffPWM(timer);
|
if (timer != NOT_ON_TIMER) turnOffPWM(timer);
|
||||||
|
|
Loading…
Reference in New Issue