digistump / DigistumpArduino

Files to add Digistump support (Digispark, Pro, DigiX) to Arduino 1.6.X (1.6.5+)
934 stars 377 forks source link

Reboot example on Wiki page is wrong #108

Open codebeat-nl opened 4 years ago

codebeat-nl commented 4 years ago

On this wiki page: https://digistump.com/wiki/digispark/tricks

You will find this example:

void reboot(void) 
{
  noInterrupts(); // disable interrupts which could mess with changing prescaler
  CLKPR = 0b10000000; // enable prescaler speed change
  CLKPR = 0; // set prescaler to default (16mhz) mode required by bootloader
  // problem starts here:
  void (*ptrToFunction)(); // allocate a function pointer
  ptrToFunction = 0x0000; // set function pointer to bootloader reset vector
  (*ptrToFunction)(); // jump to reset, which bounces in to bootloader
}

The example assumes to call something however in pointer world 0 or null points to nothing and calling this cause a null pointer exception (Google it). Because there is no exception handling the MCU will crash and cause a 'reboot'. "set function pointer to bootloader reset vector" is not true, it is a null pointer assignment. (*ptrToFunction)(); try to call the contents at a location to nothing, because the pointer points to nothing, it tries to call a non-allocated function. And, because there is nothing to call, the MCU will crash. There is no 'jump', this is dirty stuff to crash the MCU and 'restart' the bootloader. Also there is a chance the device will not reboot at all, can lead to unpredictable results. Notice that the pointer itself is in volatile memory, not flash memory.

This example of 'rebooting' the device (any AVR, others taking over the pointer part), it is everywhere, people, youngsters think this is a legit method to reboot the device however actually it is just a bad example (of pointer abuse).

bratoff commented 4 years ago

I am not a representative of Digistump, but I'd like to point out that there is an announcement on the Wiki stating that it's in "collaborative mode" and inviting users to provide additions and revisions to the Wiki. That's probably the best way to address an issue with the Wiki, rather than reporting it as a bug in the core.

ArminJo commented 4 years ago

For all youngsters: It may be not ideal, but it works most of the time, since the bootloades resets the StackPointer. And the function is called reboot() not reset().

codebeat-nl commented 4 years ago

For all youngsters: It may be not ideal, but it works most of the time, since the bootloades resets the StackPointer. And the function is called reboot() not reset().

Best name for function is crashAndHopefullyRestart(); or fingersCrossedReboot();