[PATCH 01/13] boot: Split out the first part of fit_image_load()

[Simon Glass]

Hi Tom,

On Wed, 8 Apr 2026 at 10:25, Tom Rini <trini at konsulko.com> wrote:
>
> On Wed, Mar 25, 2026 at 10:54:10AM -0600, Simon Glass wrote:
>
> > This function is over 250 lines long. Split out the image-selection
> > part into a new select_image() function which handles format checking,
> > configuration selection and image-node lookup.
> >
> > Take care to only call fit_get_name() when a valid node is found, since
> > libfdt does not handle negative offsets gracefully.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
>
> Function length isn't a problem in and of itself, especially when we
> aren't pulling out some 4 or 5 level deep nested logic to improve
> readability.

In my view the current code is very hard to maintain and change. I
know this because I have tried to do it:

1. By my count, fit_image_load() declares 20 locals. Even without deep
nesting, tracking which variables are live at which point in a
function of ~270 lines is extremely hard (see [1] for someone else
that had this problem). Just to take one example, pulling
select_image() out makes it explicit that cfg_noffset (for example) is
only relevant during image selection and doesn't leak into the
load/decompress path.

2. The function has distinct phases: format checking, configuration
selection, hash verification, and image-node lookup. So we have 'find
what to load' vs. 'load it'. These  are much easier to understand with
clean function boundaries, rather than an arbitrary split at a line
count.

3. Reviewability - when someone adds a feature to the loading path,
they currently have to read past 90 lines of selection logic to find
where their change fits. Smaller functions with clear contracts make
it easier to see where new code belongs.

Regards,
Simon

[1] https://patchwork.ozlabs.org/project/uboot/patch/20260324-boot-fit-fix-8-byte-alignement-for-overlays-v1-1-257b132b6bda@baylibre.com/