Check for valid pin properly (fixes #4605)

[cousteaulecommandant]

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.)

[NicoHood]

I talked with him on IRC. the patch (after he explained it again) makes totally sense and is essential to be applied. Otherwise a wrong pin can cause very undefined behavior. Also it now makes things simpler.

[PaulStoffregen]

It's good to see someone's trying to get this fixed again, after all these years!

Hope it can actually happen this time...

[cousteaulecommandant]

Any idea how to do this for the Due? The sam file looks quite more complicated.
Apparently sam uses a single table for everything, but unlike for avr it is defined in a .cpp file, leaving visible only an extern const PinDescription g_APinDescription[] ; declaration in the .h, so the sizeof/sizeof trick won't work here.
One possibility would be relying on PINS_COUNT working, although this would probably have the "holes" issue. Also, I've noticed that the table includes an "all pins" section at the end which is not counted in PINS_COUNT but might be useful.
Another possibility would be to create a pin_max constant in the variant.cpp file and expose it via extern.
And finally, the elegant solution (which seems to be how this table was intended to be used) would be to seek the table until reaching the valid pin or the last element (which is some sort of "null terminator"), since the table contains an ulPin field, hinting that it should be looked up, not accessed directly.

The variant.cpp file could definitely use some documentation explaining how to use it... (the fact that it's written using Hungarian notation (incorrectly, by the way) doesn't help)

[buzztiaan]

this is a excellent fix!

[tttapa]

Does anyone know why this hasn't been merged?

[facchinm]

Closing here; if someone wants to adopt this PR, please reopen it using this link https://github.com/arduino/ArduinoCore-avr/compare/pr_4649?expand=1 , thanks.