[PATCH RFC 00/10] image: Reduce the abuse of #ifdefs in image-sig.c
mr.nuke.me at gmail.com
Fri May 14 21:45:52 CEST 2021
This series is motivated by Simon's bratwurst:
[PATCH v2 00/50] image: Reduce #ifdefs and ad-hoc defines in image code
A big problem with current mkimage code, as well as the code it shares
with the targets is that it uses a lot of #ifdefs. Some of the #ifdefs
are defined based on other macros, or CONFIG_() options.
Simon's approach to fixing this is to extend Kconfig to the host-side
of mkimage, and replace ifdefs with "if (CONFIG_IS_ENABLED())"
wherever possible. This would resolve most cosmetic issues caused by
the ungodly abuse of #ifdefs.
I do not like the aforementioned approach, because I believe it is a
band-aid to a much deeper problem. I believe the fundamental problem
is the incorrect separation of target code and host code, which has
led to the accumulation of crud over the years. I believe in
refactoring the current code in order to reduce the need for decision
points and branch divergence between the host and target.
In this series I intend to demonstrate a proof-of-concept for achieving
this with respect to signing algorithms. I treat the three
image_get_*_algo() functions as an interface, and decouple the host and
target implementations. This enable a dramatic reduction of #ifdefs
decision points in image-sig.c
The existing implementation is mostly suited for the host-side, where
it is reused. On the target-side, I implement a linker-list based
array of crypto_algo structures, inspired by the DM driver lists.
Two macros are deleted, rsa.h, and ecdsa.h are completely cleaned of
#ifdefs, and the new host-side implementation of image-sig.c has
zero #ifdefs. This comes at a minimal increase in the noumber of source
lines of code.
Only image_get_crypto_algo() is implemented as a linker list in this
POC. image_get_checksum_algo() and image_get_padding_algo() would also
see healthy benefits.
Alexandru Gagniuc (10):
common: Move host-only logic in image-sig.c to separate file
common: image-sig.c: Remove host-specific logic and #ifdefs
image: Add support for placing crypto_algo in linker lists
[UNTESTED] image: Add support for relocating crypto_algos in linker
image: rsa: Move verification algorithm to a linker list
image: image-sig.c: Remove crypto_algos array
lib: ecdsa: Remove #ifdefs from ecdsa.h
lib: rsa: Remove #ifdefs from rsa.h
image: Eliminate IMAGE_ENABLE_VERIFY macro
image: Eliminate IMAGE_ENABLE_VERIFY_ECDSA macro
common/image-sig-host.c | 134 ++++++++++++++++++++++++++++++++++++++++
common/image-sig.c | 71 +++++----------------
include/image.h | 13 ++--
include/u-boot/ecdsa.h | 25 --------
include/u-boot/rsa.h | 47 --------------
lib/rsa/rsa-verify.c | 16 +++++
tools/Makefile | 2 +-
7 files changed, 172 insertions(+), 136 deletions(-)
create mode 100644 common/image-sig-host.c
More information about the U-Boot