[PATCH RFC 00/10] image: Reduce the abuse of #ifdefs in image-sig.c

Alexandru Gagniuc 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
    lists
  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

-- 
2.31.1



More information about the U-Boot mailing list