dapboot icon indicating copy to clipboard operation
dapboot copied to clipboard

Overriding target_get_force_bootloader

Open obra opened this issue 3 years ago • 4 comments

For my application, we want to inhibit the ability for software to force the bootloader to reboot into programming mode without physical user interaction. To that end, I think I need to override target_get_force_bootloader. By default, that function isn't marked as weak. Is the right change as follows? If so, I'll submit a PR. (Come to think of it, I'll want to do this with target_clock_setup, too)

modified: src/target.h
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
@@ -26,7 +26,7 @@
 extern void target_clock_setup(void);
 extern void target_gpio_setup(void);
 extern const usbd_driver* target_usb_init(void);
-extern bool target_get_force_bootloader(void);
+extern bool target_get_force_bootloader(void)  __attribute__((weak));
 extern void target_get_serial_number(char* dest, size_t max_chars);
 extern size_t target_get_max_firmware_size(void);
 extern void target_log(const char* str);

obra avatar Jan 11 '22 02:01 obra

I've only been marking functions as weak as a convenience for some of hooks that have default / no-op implementations in dummy.c.

In general, to customize the behavior for a target, I would either add more #ifdef controlled code to target_stm32f103.c or create a new target directory if the changes are not broadly applicable or too extensive to put in the common implementation.

devanlai avatar Jan 11 '22 03:01 devanlai

Just in terms of extensibility without butchering the core code, I've been finding the weak functions really nice. It's meant that I can extend the code for a weird use case without junking up the core implementation and still keep the cut-and-paste duplication to a minimum.

obra avatar Jan 11 '22 03:01 obra

Weak functions work okay if there's only one level of overriding necessary, but IIRC, it's not defined what happens if you provide multiple weak definitions of a symbol to the linker. I agree that it is nice to be able to extend things without modification, but there are only so many hacks we can get away with before we end up building a poor version of C++.

devanlai avatar Jan 11 '22 04:01 devanlai

Understood.

obra avatar Jan 11 '22 04:01 obra