All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Golle <daniel@makrotopia.org>
To: Tom Rini <trini@konsulko.com>,
	Daniel Golle <daniel@makrotopia.org>,
	Simon Glass <sjg@chromium.org>,
	Aristo Chen <aristo.chen@canonical.com>,
	Anton Ivanov <anton@binarly.io>, Vincent Jardin <vjardin@free.fr>,
	Randolph Sapp <rs@ti.com>,
	Anders Roxell <anders.roxell@linaro.org>,
	u-boot@lists.u-boot-project.org, openwrt-devel@lists.openwrt.org
Subject: [PATCH v2 00/11] imagemap: structural and cosmetic review follow-ups
Date: Wed, 21 Oct 2026 15:08:59 +0100	[thread overview]
Message-ID: <cover.1792591479.git.daniel@makrotopia.org> (raw)

The clean-up half of the v1 review of the imagemap series
(cover.1787272661.git.daniel@makrotopia.org). It applies on top of
"imagemap: fixes from the v1 review", which carries the correctness
changes and which several of these patches touch the same lines as.

Patches 1 to 4 are self-contained tidy-ups without any functional
changes: the suite list put back in alphabetical order, a derived size
for the header probe in place of a bare 64, a corrected rationale on the
imagemap_lookup() guard, and test locals hoisted out of an anonymous
block and a mid-block declaration.

Patches 5 and 6 correct the documentation: the imagemap_create()
signature, which callers a reader can actually reach, bootmeth_openwrt
described as a future consumer, the index entry sorted, and the section
on phase gating that the commit adding the file claimed was there.

Patches 7 to 10 are the structural ones which do introduce functional
changes: imagemap devices are named after the partition they cover and
hold a name driver model owns, so two mappings on one block device stay
distinct. bootm_start() sets the imagemap itself, keeping the invariant
in one function. The data-position and totalsize-alignment sequence is
shared between fit_image_get_data() and the storage path, which also
gives the latter the range check it lacked and settles the two spellings
of the alignment. Finally the two imagemap pointers compile only when
CONFIG_IMAGEMAP is on, reached through accessors that fold away when it
is off.

Patch 11 adds a new unit test Simon had asked for in his review.

The sandbox defconfig symbol ordering and the FIT_VERITY_NODENAME
rename that the v1 review also asked for are already in the tree, so
they are not separate patches in this posting.

One review point is not addressed here. Moving the imagemap out of the
global images struct and into an explicit parameter of
fit_image_get_data() means changing that function and fit_image_verify(),
both public, and their callers under arch/arm, arch/x86, arch/arm/mach-k3,
cmd/, common/ and tools/. It is also a design question rather than a
cleanup: the transparency of the verification path is exactly what lets
imagemap hook in without touching the FIT code. I would rather agree the
shape first than guess at it, so it is left out and proposed separately.

Tested on sandbox as for the first series: every commit builds, "ut
imagemap" passes 14/14 under "ut -r3", host tools build, the
documentation builds with no warnings, and a full test.py run matches
unmodified next. Built with CONFIG_IMAGEMAP=n as well, which patch 10
exists to keep working.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2: 
 - The sandbox defconfig symbol ordering and the FIT_VERITY_NODENAME
   rename that the v1 review also asked for are already in the tree, so
   the cover no longer counts them as patches in this series; they were
   not dropped between postings.
 - Patch 3: the guard rationale now says "same width" instead of
   "same rank"; on ILP32 loff_t outranks ulong and the subtraction
   stays signed, and it is the LP64 unsigned conversion that makes
   the guard essential.
 - Patch 7: the named path duplicates the caller's name directly
   instead of copying it through a fixed-size buffer that truncated
   it.
 - Patch 9: the helper is renamed fit_image_get_ext_data_offset() to
   match its siblings, fit_image_get_data() folds the embedded case
   into an early return, and the message explains the dropped
   signed-value guard.
 - Patch 10: adds bootm_info_set_imagemap(), keeps the bootm_info_*
   guards in step with bootm_imagemap(), keeps the device in a local
   in the storage path, and shares the release between the OS-GO and
   error paths in a helper.
 - Patch 11: the scenario comment no longer names the reviewer, the
   bare 64 and 256 become named constants, and the pointer
   comparison drops its casts.
 - Patches 1, 2, 4, 5, 6 and 8: no changes.

Daniel Golle (11):
  test: cmd_ut: keep the suite list alphabetical
  boot: bootm: derive the header probe size
  boot: imagemap: correct the lookup guard rationale
  test: boot: imagemap: declare locals at the top
  doc: imagemap: describe the API and phase gating
  doc: develop: sort the imagemap index entry
  boot: imagemap: name the device after its partition
  boot: bootm: set the imagemap from bootm_start()
  boot: fit: share the external-data offset calculation
  boot: build the imagemap pointers only when enabled
  test: boot: imagemap: caller-owned region not resized

 boot/bootm.c             |  55 ++++++------
 boot/image-fit.c         | 175 +++++++++++++++++++++------------------
 boot/imagemap.c          |  37 +++++++--
 doc/develop/imagemap.rst |  30 +++++--
 doc/develop/index.rst    |   2 +-
 include/bootm.h          |  31 +++++++
 include/image.h          |  31 +++++++
 test/boot/imagemap.c     | 121 +++++++++++++++++++++++----
 test/cmd_ut.c            |   4 +-
 9 files changed, 351 insertions(+), 135 deletions(-)


base-commit: e36b675ebc4c41f1f4187c46d363d581c371d538
-- 
2.56.0

             reply	other threads:[~2026-10-21 14:09 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-21 14:08 Daniel Golle [this message]
2026-10-21 14:09 ` [PATCH v2 01/11] test: cmd_ut: keep the suite list alphabetical Daniel Golle
2026-10-21 14:09 ` [PATCH v2 02/11] boot: bootm: derive the header probe size Daniel Golle
2026-10-21 14:09 ` [PATCH v2 03/11] boot: imagemap: correct the lookup guard rationale Daniel Golle
2026-10-23 11:37   ` Simon Glass
2026-10-21 14:09 ` [PATCH v2 04/11] test: boot: imagemap: declare locals at the top Daniel Golle
2026-10-21 14:09 ` [PATCH v2 05/11] doc: imagemap: describe the API and phase gating Daniel Golle
2026-10-21 14:09 ` [PATCH v2 06/11] doc: develop: sort the imagemap index entry Daniel Golle
2026-10-21 14:09 ` [PATCH v2 07/11] boot: imagemap: name the device after its partition Daniel Golle
2026-10-21 14:09 ` [PATCH v2 08/11] boot: bootm: set the imagemap from bootm_start() Daniel Golle
2026-10-21 14:09 ` [PATCH v2 09/11] boot: fit: share the external-data offset calculation Daniel Golle
2026-10-23 11:37   ` Simon Glass
2026-10-21 14:10 ` [PATCH v2 10/11] boot: build the imagemap pointers only when enabled Daniel Golle
2026-10-23 11:37   ` Simon Glass
2026-10-21 14:10 ` [PATCH v2 11/11] test: boot: imagemap: caller-owned region not resized Daniel Golle
2026-10-23 11:37   ` Simon Glass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1792591479.git.daniel@makrotopia.org \
    --to=daniel@makrotopia.org \
    --cc=anders.roxell@linaro.org \
    --cc=anton@binarly.io \
    --cc=aristo.chen@canonical.com \
    --cc=openwrt-devel@lists.openwrt.org \
    --cc=rs@ti.com \
    --cc=sjg@chromium.org \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.u-boot-project.org \
    --cc=vjardin@free.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.